On Wed, 19 May 2021 14:25:41 GMT, Richard Reingruber <rr...@openjdk.org> wrote:

> Just one more, rather unimportant comment...
> 
> Either way: LGTM!
> 
> Thanks, Richard.

Thanks!

> src/hotspot/share/prims/jvmtiRawMonitor.cpp line 382:
> 
>> 380: 
>> 381:     _recursions = 0;
>> 382:     ret = simple_wait(self, millis);
> 
> IMHO the guarantee at L379 is redundant with the newly added identical 
> guarantee in `JvmtiRawMonitor::simple_wait()` at L240.
> 
> In case you agree to remove the guarantee, I don't see why the following 
> statements cannot be pulled out of the if-statement.
> 
>     _recursions = 0;
>     ret = simple_wait(self, millis);
>     _recursions = save;

Yes, it can be re-written as something below, but is this more readable?


  _recursions = 0;                                                              
                                 
  ret = simple_wait(self, millis);                                              
                                     
  
  if (self->is_Java_thread()) { // JavaThread re-enter                          
                                                              
    JavaThread* jt = self->as_Java_thread();                                    
                                       
    {                                                                           
                                       
      ThreadInVMfromNative tivmfn(jt);                                          
                                       
      for (;;) {                                                                
                                       
        ExitOnSuspend eos(this);                                                
                                       
        {                                                                       
                                       
          ThreadBlockInVMPreprocess<ExitOnSuspend> tbivmp(jt, eos);             
                                       
          simple_enter(jt);                                                     
                                       
        }                                                                       
                                       
        if (!eos.monitor_exited()) {                                            
                                       
          break;                                                                
                                       
        }                                                                       
                                       
      }                                                                         
                                       
      if (jt->is_interrupted(true)) {                                           
                                       
        ret = M_INTERRUPTED;                                                    
                                       
      }                                                                         
                                       
    }                                                                           
                                       
  } else { // Non-JavaThread re-enter                                           
                      
    assert(ret != M_INTERRUPTED, "Only JavaThreads can be interrupted");        
                                       
    simple_enter(self);                                                         
                                   
  }
  _recursions = save;

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

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

Reply via email to