RFR: 8264411: serviceability/jvmti/HeapMonitor tests intermittently fail due to large TLAB size [v2]

[Chris Plummer]

On Tue, 30 Mar 2021 23:51:41 GMT, Hui Shi <hshi at openjdk.org> wrote:

>> …ue to large TLAB size
>> 
>> serviceability/jvmti/HeapMonitor tests intermittently fail when using PS/Serial GC, original test has implicit assumptions on TLAB size and depends on allocate fix amount of objects to consume TLAB and trigger object sampling. These tests will fail if TLAB is above 20M (this can easily happen when using PS/Serial GC and heap is large), when allocation can not consume current TLAB and _byte_until_sample.
>> 
>> Fix in tests is adding an explicit GC to consume current TLAB.
>> Running on 256G memory machine, make run-test CONF=release TEST="test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/" 'JTREG=JOBS=12;VM_OPTIONS=-XX:ActiveProcessorCount=1'
>> 
>> before fix: 6 or 7 tests in 20 tests intermittently fail
>> after fix: no failure in 100 runs release/fastdebug
>> 
>> This might also fix https://bugs.openjdk.java.net/browse/JDK-8225313
>
> Hui Shi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update copyright year

Changes requested by cjplummer (Reviewer).

test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java line 176:

> 174:     //    and set _bytes_until_sample to 0.
> 175:     //    initial _bytes_until_sample is geometric variable with the specified mean
> 176:     //    (512K by default), check ThreadHeapSampler::pick_next_geometric_sample()

I think there is too much detail about the hotspot implementation in these comments. I think for (1) it is enough to say that the current TLAB needs to be consumed, and that this can be accomplished with a System.gc(). For (2) I don't really understand what you are saying. If you can explaining without referring to specific fields in hotspot, that would be better. I also don't understand how you are making sure that (2) is triggered. Does that happen during the allocation loop?

test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java line 180:

> 178:     // trigger GC to consume current TLAB in step1
> 179:     // consume initial _bytes_until_sample in following loops, each iteration consume
> 180:     // about 1600KB, 10 iterations can definitly consume intitial _bytes_until_sample

I think this comment should be after the System.gc() call.

test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java line 215:

> 213:       enableSamplingEvents();
> 214:     }
> 215:     // similar reason with sampleEverything, consume TLAB and trigger sample

How about:
`    // Use System.gc() to consume TLAB and trigger sampling as described above in sampleEverything`

test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatObjectCorrectnessTest.java line 49:

> 47: 
> 48:     HeapMonitor.enableSamplingEvents();
> 49:     // retire TLAB with GC, ensure sample happens, not assume TLAB size

// Instead of relying on the allocation loop to fill and retire the TLAB, which might not happen,
// use System.gc() to retire the TLAB and ensure sampling happens

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

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