RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v7]
Daniel D.Daugherty
dcubed at openjdk.java.net
Wed Sep 9 23:30:54 UTC 2020
On Wed, 9 Sep 2020 23:24:26 GMT, Daniel D. Daugherty <dcubed at 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
More information about the serviceability-dev
mailing list