RFR: 8348402: PerfDataManager stalls shutdown for 1ms

[Patricio Chilano Mateo]

On Fri, 24 Jan 2025 09:27:07 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> Found this when studying Leyden performance. [JDK-8049304](https://bugs.openjdk.org/browse/JDK-8049304) added 1ms sleep on destruction path to catch up with threads updating the counters. 
> 
> I was not able to confidently prove the deletion race is benign. Even though `sleep` is not really a fix for that race either, I think it is safer to use GlobalCounter to coordinate deletions.
> 
> This improves startup (roundtrip) tests for the expected 1ms:
> 
> 
> $ hyperfine -w 10 -r 100 ...
> 
> # Before
> Benchmark 1: build/linux-x86_64-server-release/images/jdk/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseEpsilonGC -Xmx128m Hello
>   Time (mean ± σ):      20.0 ms ±   0.3 ms    [User: 11.7 ms, System: 16.0 ms]
>   Range (min … max):    19.0 ms …  21.7 ms    1000 runs
> 
> # After
> Benchmark 1: build/linux-x86_64-server-release/images/jdk/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseEpsilonGC -Xmx128m Hello
>   Time (mean ± σ):      19.0 ms ±   0.3 ms    [User: 11.9 ms, System: 15.8 ms]
>   Range (min … max):    18.2 ms …  20.8 ms    1000 runs
> 
> 
> Additional testing:
>  - [x] Linux AArch64 server fastdebug, `all`
>  - [x] Linux x86_64 server fastdebug, `all`

If we are going to remove the objectMonitor counters not sure if it makes sense to change `PerfDataManager::destroy()` since that workaround is specific to those cases (added in 8049304). So either other users of PerfData counters don’t need this or we would be missing the use of `_has_PerfData` and `GlobalCounter::CriticalSection` in other places.

There is also an alternative simpler fix. The problem is in the increment of `FutileWakeups` in `ObjectMonitor::EnterI()` because the thread is in a safepoint safe state, so the check of `SafepointSynchronize::is_at_safepoint()` in `perfMemory_exit()` is not enough to make sure no JavaThread is accessing the counters when freeing the memory. But we could increment a local variable instead (passed as argument or returned by EnterI), and then add it to `FutileWakeups` if we exit the `ThreadBlockInVMPreprocess` scope. There is a similar instance in `ObjectMonitor::ReenterI()` but we are not safepoint safe in that case (comment is not accurate).

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

PR Comment: https://git.openjdk.org/jdk/pull/23293#issuecomment-2622496973