RFR: 8212107: VMThread issues and cleanup [v3]

[Robbin Ehn]

On Wed, 23 Sep 2020 12:51:48 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update after Coleen and David
>
> src/hotspot/share/runtime/vmThread.cpp line 389:
> 
>> 387:     if (!prev_vm_operation->allow_nested_vm_operations()) {
>> 388:       fatal("Nested VM operation %s requested by operation %s",
>> 389:             op->name(), vm_operation()->name());
> 
> This is better as `prev_vm_operation`, not `vm_operation()`? Saves some brain cycles reading...

I changed prev_vm_operation to _cur_vm_operation and removed uses of vm_operation() to be consistent with the rest of
the method (storing and reading from _cur_vm_operation).

  VM_Operation* prev_vm_operation = NULL;
  if (_cur_vm_operation!= NULL) {
    // Check the VM operation allows nested VM operation.
    // This normally not the case, e.g., the compiler
    // does not allow nested scavenges or compiles.
    if (!_cur_vm_operation->allow_nested_vm_operations()) {
      fatal("Nested VM operation %s requested by operation %s",
            op->name(), _cur_vm_operation->name());
    }
    op->set_calling_thread(_cur_vm_operation->calling_thread());
    prev_vm_operation = _cur_vm_operation;
  }

> src/hotspot/share/runtime/vmThread.cpp line 483:
> 
>> 481:       break;
>> 482:     }
>> 483:   } while(!should_terminate());
> 
> So now we have the duplicated `should_terminate()` here. Can we check it once (logically)? For example:
> 
> do {
>   wait_for_operation();
>   assert(_next_vm_operation != NULL, "Must have one");
>   inner_execute(_next_vm_operation);
> } while (!should_terminate());
> 
> If not, then it might be rewritten without change in logic this:
> 
> while (true) {
>   if (should_terminate()) break;
>   wait_for_operation();
>   if (should_terminate()) break;
>   assert(_next_vm_operation != NULL, "Must have one");
>   inner_execute(_next_vm_operation);
> }

If we return from wait_for_operation() due to should_terminate() but someone just stored an op into _next_vm_operation.
Calling inner_execute() would execute that op before exiting, so I'm going with an additional if on should_terminate().

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

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