On Wed, 5 Feb 2025 14:41:39 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> Here is an attempt to simplify GCLocker implementation for Serial and >> Parallel. >> >> GCLocker prevents GC when Java threads are in a critical region (i.e., >> calling JNI critical APIs). JDK-7129164 introduces an optimization that >> updates a shared variable (used to track the number of threads in the >> critical region) only if there is a pending GC request. However, this also >> means that after reaching a GC safepoint, we may discover that GCLocker is >> active, preventing a GC cycle from being invoked. The inability to perform >> GC at a safepoint adds complexity -- for example, a caller must retry >> allocation if the request fails due to GC being inhibited by GCLocker. >> >> The proposed patch uses a readers-writer lock to ensure that all Java >> threads exit the critical region before reaching a GC safepoint. This >> guarantees that once inside the safepoint, we can successfully invoke a GC >> cycle. The approach takes inspiration from `ZJNICritical`, but some >> regressions were observed in j2dbench (on Windows) and the micro-benchmark >> in [JDK-8232575](https://bugs.openjdk.org/browse/JDK-8232575). Therefore, >> instead of relying on atomic operations on a global variable when entering >> or leaving the critical region, this PR uses an existing thread-local >> variable with a store-load barrier for synchronization. >> >> Performance is neutral for all benchmarks tested: DaCapo, SPECjbb2005, >> SPECjbb2015, SPECjvm2008, j2dbench, and CacheStress. >> >> Test: tier1-8 > > Albert Mingkun Yang has updated the pull request with a new target base due > to a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Merge branch 'master' into gclocker > - review > - Merge branch 'master' into gclocker > - gclocker I like the direction this is taking us, but I think we could go even further and eventually fold the JNI critical region into the existing safepoint mechanism. To me, the safepoint mechanism already implements a readers-writer lock, with threads states like _thread_in_Java/_thread_in_vm already being "critical regions". With this change, we have two nested readers-writer locks that a GC needs to acquire. I think if we made entering and exiting a JNI critical region change the thread state, (probably by introducing a new thread state), then we don't need a separate readers-writer lock for JNI critical region. However, maybe we don't want to go that far, as the current solution allows us GC-specific implementations and allows each different GC VMOp to decide if it needs to block for JNI critical regions. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23367#issuecomment-2637865869
