RFR(S/L): 8230184 rename, whitespace, indent and comments changes in preparation for lock free Monitor lists

Kim Barrett kim.barrett at oracle.com
Wed Aug 28 01:04:17 UTC 2019

> On Aug 27, 2019, at 6:29 PM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
> Greetings,
> This is my first set of JDK14 baseline cleanup changes extracted from the
> Async Monitor Deflation project (JDK-8153224).
> This is an "S" review because it is conceptually Small. It is also an "L"
> review because it has a mind numbingly Large number of cleanup changes.
> Coleen, should appreciate this one since it takes care of a number of
> naming issues that she (and others) have wanted to see fixed for years.
> As usual, the bug report has a yacky description of what I'm cleaning up
> this round and why:
>    JDK-8230184 rename, whitespace, indent and comments changes in preparation
>                for lock free Monitor lists
>    https://bugs.openjdk.java.net/browse/JDK-8230184
> Here's the webrev URL:
>     http://cr.openjdk.java.net/~dcubed/8230184-webrev/0_for_jdk14/
> This webrev is relative to jdk-14+11.
> This fix is included in my latest round of JDK-8153224 testing which
> included Mach5 Tier[1-8], my stress kit on Linux-X64 and Solaris-X64,
> my Inflate2 test running in parallel with Kitchensink8H on macOSX,
> Linux-X64 and Solaris-X64. I also ran the Inflate2 and Kitchensink8H
> combo in parallel on the jdk-14+11 baseline + this fix on macOSX,
> Linux-X64 and Solaris-X64. No regressions were observed in any of
> the testing.
> Thanks, in advance, for any comments, suggestions or questions.
> Dan

Some variables I expected to be renamed but weren't:

 167   ObjectWaiter* volatile _EntryList;  // Threads blocked on entry or reentry.
 173   Thread* volatile _Responsible;
 175   volatile int _Spinner;            // for exit->spinner handoff optimization
 176   volatile int _SpinDuration;
 182   ObjectWaiter* volatile _WaitSet;  // LL of threads wait()ing on the monitor
 185   volatile int _WaitSetLock;        // protects Wait Queue - simple spinlock

Similarly for the block of private functions at the end of the class. 

Maybe some of these aren't surviving future work, so not worth
renaming now?  But then why mess with whitespace and parameter names
now?  So that's probably not the rationale.

1013 static void InduceScavenge(Thread* self, const char * Whence) {

Whence parameter name and type formatting not updated.

Making sure we're reading carefully? :)


Looks good.

I want to say it's trivial, but maybe it's too mind numbingly large.

More information about the hotspot-runtime-dev mailing list