RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v7]

Daniel D.Daugherty dcubed at openjdk.java.net
Wed Sep 9 23:26:50 UTC 2020


On Wed, 9 Sep 2020 14:06:02 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reverted to the original code as the explicit assertion is preferred.
>
> 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!

-------------

PR: https://git.openjdk.java.net/jdk/pull/37


More information about the serviceability-dev mailing list