RFR: 8254029: ObjectMonitor cleanup/minor bug-fix changes extracted from JDK-8253064

[David Holmes]

On Tue, 13 Oct 2020 20:31:16 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

> 8254029: ObjectMonitor cleanup/minor bug-fix changes extracted from JDK-8253064

Hi Dan,

Overall this looks good to me.

A few minor comments/queries below.

Thanks,
David

src/hotspot/share/runtime/vmOperations.cpp line 458:

> 456:   // The ObjectMonitor subsystem uses perf counters so do this before
> 457:   // we call exit_globals() so we don't run afoul of perfMemory_exit().
> 458:   ObjectSynchronizer::do_final_audit_and_print_stats();

Given this used to be done in the prolog, is there anything above this line that might interfere with it somehow? Or
more simply put, why not do this first so that it is as close as possible to where it used to be?

src/hotspot/share/runtime/synchronizer.cpp line 2949:

> 2947:   assert(Thread::current()->is_VM_thread(), "sanity check");
> 2948:
> 2949:   if (is_final_audit()) {  // Only do the audit once.

It seems impossible that you could ever call this twice. A simple file static flag for a debug build would be simpler
if you just wanted to assert that.

src/hotspot/share/runtime/vmThread.cpp line 194:

> 192:   // we signal that the VM thread is gone. We don't want to run afoul
> 193:   // of perfMemory_exit() in exit_globals().
> 194:   ObjectSynchronizer::do_final_audit_and_print_stats();

Again I wonder why this was moved so far from the original call to deflate? It makes sense to do the final deflation
once the final safepoint has been reached, but should we do it first once the safepoint has been reached?

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

Marked as reviewed by dholmes (Reviewer).

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