RFR: 8212107: VMThread issues and cleanup [v2]

[Robbin Ehn]

On Wed, 23 Sep 2020 11:02:54 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> inner_execute(..) is called in the non nested-case here:
>> https://github.com/openjdk/jdk/blob/e49178a4445378fa0b5505ad6e9f1661636f88b8/src/hotspot/share/runtime/vmThread.cpp#L474
>> 
>> Nested case:
>> https://github.com/openjdk/jdk/blob/e49178a4445378fa0b5505ad6e9f1661636f88b8/src/hotspot/share/runtime/vmThread.cpp#L511
>
> Sorry I missed that. Seems odd that inner_execute handles nesting when that is only possible via one of the paths by
> which it is called - that's why I thought it was only for the case where called from execute(). I'd rather see the
> nesting logic handled as before, exclusively on the code path in which it can occur.

That would create a lot of code duplication:
void VMThread::none_nested_inner_execute(VM_Operation* op) {
  Thread* current = Thread::current();
  assert(current->is_VM_thread(), "must be a VM thread");

  _cur_vm_operation = op;

  HandleMark hm(VMThread::vm_thread());
  EventMark em("Executing %s VM operation: %s", op->name());

  // If we are at a safepoint we will evaluate all the operations that
  // follow that also require a safepoint
  log_debug(vmthread)("Evaluating %s %s VM operation: %s",
                      _cur_vm_operation->evaluate_at_safepoint() ? "safepoint" : "non-safepoint",
                      _cur_vm_operation->name());

  bool end_safepoint = false;
  if (_cur_vm_operation->evaluate_at_safepoint()) {
    SafepointSynchronize::begin();
    if (_timeout_task != NULL) {
      _timeout_task->arm();
    }
    end_safepoint = true;
  }

  evaluate_operation(_cur_vm_operation);

  if (end_safepoint) {
    if (_timeout_task != NULL) {
      _timeout_task->disarm();
    }
    SafepointSynchronize::end();
  }

  _cur_vm_operation = NULL;
}
Which 80% the same. (Same minus a few lines)

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

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