RFR: 8226416: MonitorUsedDeflationThreshold can cause repeated async deflation requests [v4]

[Daniel D.Daugherty]

On Wed, 13 Jan 2021 02:04:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update comments for monitors_used_above_threshold() policy to include info about no-progress async monitor deflation cycles and the NoAsyncDeflationProgressMax option.
>
> src/hotspot/share/runtime/synchronizer.cpp line 1170:
> 
>> 1168:     float remainder = (100.0 - MonitorUsedDeflationThreshold) / 100.0;
>> 1169:     size_t new_ceiling = ceiling + (ceiling * remainder) + 1;
>> 1170:     size_t old_ceiling = ObjectSynchronizer::in_use_list_ceiling();
> 
> You could capture:
> 
> size_t old_ceiling = ceiling;
> 
> back after line 1157, rather than re-reading it.

Fixed.

> test/hotspot/jtreg/runtime/Monitor/MonitorUsedDeflationThresholdTest.java line 118:
> 
>> 116:             if (too_many == null) {
>> 117:                 throw new RuntimeException("Did not find too_many string in output.\n");
>> 118:             }
> 
> You should call output_detail.reportDiagnosticSummary() before throwing the exception. That emulates what OutputAnalyzer.shouldContain would do. (or you could just use shouldContain).

I added a call to output_detail.reportDiagnosticSummary().
I didn't use OutputAnalyzer.shouldContain() intentionally
because of this output line:

`System.out.println("too_many='" + too_many + "'");`

which is there for diagnostic purposes.

> test/hotspot/jtreg/runtime/Monitor/MonitorUsedDeflationThresholdTest.java line 64:
> 
>> 62:         }
>> 63: 
>> 64:         Object obj = new Object();
> 
> This is a thread-local object, which means the synchronization has no affect, so I think C2 could elide the following sync-block. Using a static field would make it less likely that this can happen.

Fixed.

> test/hotspot/jtreg/runtime/Monitor/MonitorUsedDeflationThresholdTest.java line 124:
> 
>> 122:             return;
>> 123:         }
>> 124: 
> 
> Please add a comment e.g.
> `// else we are the exec'd java subprocess, so run the actual test

Added.

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

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