JDK-8217658 : baseline_cleanups from Async Monitor Deflation project
  • Type: Sub-task
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 13
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2019-01-23
  • Updated: 2019-02-04
  • Resolved: 2019-01-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 13
13 b06Fixed
Description
This sub-task captures updates to the baseline code that are not
directly related to Async Monitor Deflation. 

The cleanups include:

- comment corrections and clarifications, fix typos
- implied boolean expression corrections
- delete/fix whitespace/blank lines
- remove unecessary "ObjectSynchronizer::" prefix from calls
- add additional info to existing asserts; this info was useful during
  my debugging of the Async Monitor Deflation code port.
- use a consistent style for Thread::muxAcquire() id tags
- ObjectSynchronizer::inflate_helper() return value is always
  ignore by callers so remove it.
- add missing "else" to logical if-else-statement in
  ObjectSynchronizer::get_lock_owner()
- guarantee that header() is NULL when
  ObjectSynchronizer::omRelease() is called
- code cleanup/motion in ObjectSynchronizer::omFlush()
  - cleanup guarantee() and assert() calls
  - move some field resets to NULL to when they are needed
    and group them together
  - reorder global and per-thread code blocks to be the same
    instead of arbitrarily different
  - move some per-thread code out of the gListLock critical
    section since that lock protects the global monitor list
- update existing 'monitorinflation' logging to:
  - be clear about the originating source of the log mesg
  - use 'Self' in ResourceMark ctrs which is more efficient
- Replace a set_owner(NULL) call with a set_header(NULL)
  call in the "CASE: neutral" part of ObjectSynchronizer::inflate()
  when the object->cas_set_mark() fails. This is a rare race.
- Update runtime/logging/MonitorInflationTest.java to match the
  modified output.

In all of the above cleanups, there are only three real logic changes:

- add missing "else" to logical if-else-statement in
  ObjectSynchronizer::get_lock_owner()
- guarantee that header() is NULL when
  ObjectSynchronizer::omRelease() is called
- Replace a set_owner(NULL) call with a set_header(NULL)
  call in the "CASE: neutral" part of ObjectSynchronizer::inflate()
  when the object->cas_set_mark() fails. This is a rare race.

The first logic change is simply more efficient. The second and
third logic changes are related to not leaving a bad or stale header
value in a free ObjectMonitor. I have not been able to hypothesize
any bad effects from this stale header value, but by cleaning it up,
the upcoming ObjectMonitor audit code is more strict.
Comments
I chased the history of basicLock.cpp line to: $ sgv -r1.24.3.1 src/share/vm/runtime/synchronizer.cpp | grep 'The next line appears to do nothing!' 1.24.3.1 1.24.3.1 // [RGV] The next line appears to do nothing! $ sp -r1.24.3.1 src/share/vm/runtime/synchronizer.cpp src/share/vm/runtime/SCCS/s.synchronizer.cpp: D 1.24.3.1 99/11/30 14:47:32 bobv 71 58 00006/00005/00630 MRs: COMMENTS: Hotspot64 First Delta so something from 19+ years ago. For src/hotspot/share/prims/jvmtiExport.cpp, I searched the repo for 'montior' and we don't have any other instances of that pattern. So no tests rely on that pattern.
25-01-2019

A couple of additional cleanups from David H. These are unrelated to the Async Monitor Deflation project, but this bug serves as a convenient means of getting these things fixed: On 1/23/19 11:18 PM, David Holmes wrote: > > One suggested addition to your cleanup > > ./share/runtime/basicLock.cpp > > // [RGV] The next line appears to do nothing! > intptr_t dh = (intptr_t) displaced_header(); > dest->set_displaced_header(displaced_header()); On 1/3/12 10:03 PM, David Holmes wrote: > Hi Dan, > > Spotted the typos below and thought you might want to add them to your cleanup list for next time you update this file. > > Cheers, > David > ------ > > bus2001029 /java/embedded/users/dh198349/dev-work/hotspot/src > grepsrc.sh montior > ./share/vm/prims/jvmtiExport.cpp: ("JVMTI [%s] montior contended enter event triggered", > ./share/vm/prims/jvmtiExport.cpp: ("JVMTI [%s] montior contended entered event triggered", > ./share/vm/prims/jvmtiExport.cpp: ("JVMTI [%s] montior wait event triggered", > ./share/vm/prims/jvmtiExport.cpp: ("JVMTI [%s] montior waited event triggered",
25-01-2019