On Wed, 9 Sep 2020 23:24:26 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> Marked as reviewed by kbarrett (Reviewer). > > This is a really nice set of cleanup changes. > > I have a few comments. > > https://openjdk.github.io/cr/?repo=jdk&pr=37&range=06#frames-33 > 51 if (thread->is_Java_thread()) > 52 return thread->as_Java_thread()->thread_state() == _thread_in_vm; > 53 else > 54 return thread->is_VM_thread(); > nit - this code needs braces. Not your bug, but since you've touched > this > code, do you mind fixing it? > > Note: I included the link the webrev had me on, but I have no idea what > file that is... > > https://openjdk.github.io/cr/?repo=jdk&pr=37&range=06#frames-80 > > 276 guarantee(current == get_thread() || current == > get_thread()->active_handshaker(), > 277 "must be current thread or direct handshake"); > > nit - the indent on L277 looks wrong in the webrev, but it looks right in > this paste. I don't know which is telling the truth. > > https://openjdk.github.io/cr/?repo=jdk&pr=37&range=06#frames-101 > > 358 this->as_Java_thread()->set_stack_overflow_limit(); > 359 this->as_Java_thread()->set_reserved_stack_activation(stack_base()); > nit - do you really need 'this->' here? > > https://openjdk.github.io/cr/?repo=jdk&pr=37&range=06#frames-107 > > old code: > 2074 if (thread_id == 0) { > 2075 // current thread > 2076 if (THREAD->is_Java_thread()) { > 2077 return ((JavaThread*)THREAD)->cooked_allocated_bytes(); > 2078 } > 2079 return -1; > > new code: > 2074 if (thread_id == 0) { // current thread > 2075 return thread->cooked_allocated_bytes(); > > If the calling thread is not a JavaThread and passes a thread_id ==0, > I don't think the returns are equivalent. > > The craziness in the JavaThread::pd_get_top_frame() functions made my head > hurt. > Thanks for cleaning that up! I don't see how to add myself as a reviewer... what am I missing? ------------- PR: https://git.openjdk.java.net/jdk/pull/37
