Avoid redundant Java GC bridge graph computation - #131764
Conversation
On Android the GC bridge hands a set of cross references to the client on every collection that finds at least one dead Java peer. The client answers each of those with an explicit ART collection, and while that runs the UI thread blocks creating JNI global references, which shows up as dropped frames. Two changes: Skip the SCC computation when it is known to be discarded. The VM already frees the arguments without forwarding them when the client is still processing a previous set, so the Tarjan pass and the allocations feeding it were pure waste. The GC can now ask the client via the new IGCToCLR::IsClientBridgeProcessingActive, so it skips the work instead. This is behavior neutral. Throttle how often a fresh set of cross references is produced, controlled by the new GCBridgeMinIntervalMs / System.GC.BridgeMinIntervalMs knob, defaulting to 50 ms. Only gen0 collections are throttled: bridge objects are promoted whether or not they were handed to the client, so a deferred object cannot be registered again by a later gen0 collection, and leaving gen1 and gen2 unthrottled bounds how long a dead peer can go unreported to the next gen1 collection. Deferring in this way is the same shape as what already happens when a collection lands while the client is busy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 91dd8b98-73db-41b0-8d8c-9e1c21b53f5f
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
@jkotas @janvorli @vitek-karas I pushed this as a draft to get your reactions. It seems reasonable but you may want something else. |
There was a problem hiding this comment.
Pull request overview
This PR updates the CoreCLR GC↔EE bridge plumbing (Java marshal bridge) to (1) avoid computing bridge cross-reference work that would be discarded anyway, and (2) introduce a gen0-only throttle for how frequently bridge processing requests are produced.
Changes:
- Adds
IGCToCLR::IsClientBridgeProcessingActive()(EE interface major version bump 4→5) so the GC can skip building cross references when the client is already busy. - Introduces
GCBridgeMinIntervalMs/System.GC.BridgeMinIntervalMsand uses it to throttle gen0 bridge request production. - Wires the new method across CoreCLR VM, NativeAOT, standalone GC forwarding, and the GC sample stub.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/gcenv.ee.cpp | Implements GCToEEInterface::IsClientBridgeProcessingActive() for CoreCLR VM builds. |
| src/coreclr/nativeaot/Runtime/interoplibinterface.h | Extends NativeAOT Java marshal interface with IsGCBridgeActive(). |
| src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp | Implements JavaMarshalNative::IsGCBridgeActive() by reading g_GCBridgeActive. |
| src/coreclr/nativeaot/Runtime/gcenv.ee.cpp | Implements NativeAOT GCToEEInterface::IsClientBridgeProcessingActive(). |
| src/coreclr/gc/sample/gcenv.ee.cpp | Adds a stub IsClientBridgeProcessingActive() implementation for the GC sample. |
| src/coreclr/gc/objecthandle.cpp | Gates ProcessBridgeObjects() behind ShouldProcessBridgeObjects(condemned). |
| src/coreclr/gc/gcinterface.h | Bumps EE_INTERFACE_MAJOR_VERSION to 5. |
| src/coreclr/gc/gcinterface.ee.h | Adds new IGCToCLR pure virtual IsClientBridgeProcessingActive(). |
| src/coreclr/gc/gcenv.ee.standalone.inl | Version-guards and forwards IsClientBridgeProcessingActive() for standalone GC. |
| src/coreclr/gc/gcconfig.h | Adds the new GCBridgeMinIntervalMs configuration knob (with public name). |
| src/coreclr/gc/gcbridge.h | Declares ShouldProcessBridgeObjects(uint32_t condemned). |
| src/coreclr/gc/gcbridge.cpp | Implements throttling + “client busy” skip logic and timestamps last request time. |
| src/coreclr/gc/env/gctoeeinterface.standalone.inl | Implements the new IGCToCLR method in the standalone shim class. |
| src/coreclr/gc/env/gcenv.ee.h | Declares GCToEEInterface::IsClientBridgeProcessingActive(). |
|
Tagging subscribers to this area: @anicka-net, @dotnet/gc |
Are we going to do something about this part? With this PR, we can be still triggering full ART collection every 50ms. Full GC every 50ms still sounds like a problem. |
We definitely should do something and that would need to happen in dotnet/android. It's very possible we could do enough there and not need any runtime changes. |
|
What if we just don't build SCCs or invoke ART on gen0 collections? Gen0 collections tend to be small anyway -- maybe it's fine to float? We could conservatively mark the closure and promote to gen1 even after it's CLR-unreachable, so we would implicitly put more pressure on gen1 and that would basically put generational-style pressure into the system. |
|
@BrzVlad please take a look as well |
|
Seemingly mono has this problem a lot less - do we understand why? How is mono behaving differently from CoreCLR in this case? |
|
Was this tested on the actual repro provided by the user ? Locally I'm seeing a collection every 6 seconds or so. So this change would have no effect. |
|
@steveisok Could you reduce this PR so it only addresses the redundant computation of bridge graph. It is independent of the timing heuristic and we could have it merged right away. |
Sure, I was waiting to see where our collective analysis lands but happy to reduce it now. |
Reduce this change to only the redundant bridge graph computation fix. Removes the GCBridgeMinIntervalMs / System.GC.BridgeMinIntervalMs knob and the elapsed-time check in ShouldProcessBridgeObjects, leaving only the IsClientBridgeProcessingActive() query that skips the Tarjan pass when the client would discard its result anyway. ShouldProcessBridgeObjects no longer needs the condemned generation, so the parameter is gone. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
steveisok
changed the title
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
On Android the GC bridge hands a set of cross references to the client on every collection that finds at least one dead Java peer. The client answers each of those with an explicit ART collection, and while that runs the UI thread blocks creating JNI global references, which shows up as dropped frames. This skips the SCC computation when it is known to be discarded. The VM already frees the `MarkCrossReferencesArgs` without forwarding them when the client is still processing a previous set, so the Tarjan pass and the allocations feeding it were pure waste. The GC can now ask the client first via the new `IGCToCLR::IsClientBridgeProcessingActive`, and skips the work instead. This is behavior neutral: every registered bridge object is promoted whether or not the cross references were computed, so skipping only delays reporting a dead peer, it never collects one early. Adding a method to `IGCToCLR` is a breaking interface change, so `EE_INTERFACE_MAJOR_VERSION` goes from 4 to 5. Part of the investigation in dotnet#131370. Scoped down from the original version of this PR at @BrzVlad's request — the timing heuristic that was bundled here has been removed and is not part of this change. ### Validation Built `clr` clean (0 errors, 0 warnings) for: - `android-arm64` Release — the configuration where `FEATURE_JAVAMARSHAL` is on by default - `osx-arm64` Checked — non-Android, `FEATURE_JAVAMARSHAL` also on, covers the interface plumbing outside Android `gcbridge.cpp` was confirmed compiled into both the CoreCLR VM and the NativeAOT workstation/server GC objects. The standalone GC sample under `src/coreclr/gc/sample` is not part of either build and was not compiled. > [!NOTE] > Portions of this pull request description were generated with GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 91dd8b98-73db-41b0-8d8c-9e1c21b53f5f
On Android the GC bridge hands a set of cross references to the client on every collection that finds at least one dead Java peer. The client answers each of those with an explicit ART collection, and while that runs the UI thread blocks creating JNI global references, which shows up as dropped frames.
This skips the SCC computation when it is known to be discarded. The VM already frees the
MarkCrossReferencesArgswithout forwarding them when the client is still processing a previous set, so the Tarjan pass and the allocations feeding it were pure waste. The GC can now ask the client first via the newIGCToCLR::IsClientBridgeProcessingActive, and skips the work instead. This is behavior neutral: every registered bridge object is promoted whether or not the cross references were computed, so skipping only delays reporting a dead peer, it never collects one early.Adding a method to
IGCToCLRis a breaking interface change, soEE_INTERFACE_MAJOR_VERSIONgoes from 4 to 5.Part of the investigation in #131370.
Scoped down from the original version of this PR at @BrzVlad's request — the timing heuristic that was bundled here has been removed and is not part of this change.
Validation
Built
clrclean (0 errors, 0 warnings) for:android-arm64Release — the configuration whereFEATURE_JAVAMARSHALis on by defaultosx-arm64Checked — non-Android,FEATURE_JAVAMARSHALalso on, covers the interface plumbing outside Androidgcbridge.cppwas confirmed compiled into both the CoreCLR VM and the NativeAOT workstation/server GC objects. The standalone GC sample undersrc/coreclr/gc/sampleis not part of either build and was not compiled.Note
Portions of this pull request description were generated with GitHub Copilot.