Reduce weak ref blocking with java interop - #131952
Conversation
|
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. |
|
This still needs some polishing, but I'm looking for feedback whether it would be feasible to borrow a per object bit from somewhere. Seems doable from the object header, but according to the comment there might be some friction with debug builds. cc @jkotas |
|
Tagging subscribers to this area: @anicka-net, @dotnet/gc |
There was a problem hiding this comment.
Pull request overview
This PR reduces blocking when resolving weak-reference targets during Android Java GC bridge processing by marking bridge objects that are pending client processing using a previously-unused object header bit. Weak-reference resolution only waits (returns “need to wait”) for objects currently marked as bridge-pending, instead of for all weak handles while bridge processing is active.
Changes:
- Repurposes the high syncblock/header bit as
BIT_SBLK_BRIDGE_PENDINGand uses it to mark bridge objects awaiting Java-side processing. - Tracks “pending bridge” handle cells during bridge graph construction and clears the pending bit when bridge processing completes (or is not triggered).
- Updates weak-handle fast-path (
GCHandleInternalTryGetBridgeWait) to consult bridge-pending state rather than unconditionally blocking when the bridge is active.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/syncblk.h | Renames/reassigns the previously-unused header bit to BIT_SBLK_BRIDGE_PENDING. |
| src/coreclr/vm/marshalnative.cpp | Routes weak-handle “try get” through the new bridge-pending-aware helper. |
| src/coreclr/vm/interoplibinterface.h | Adds Interop::TryGetObjectFromHandleWithoutBridgeWait declaration (FEATURE_JAVAMARSHAL). |
| src/coreclr/vm/interoplibinterface_java.cpp | Implements pending-bit check/clear for CoreCLR Java bridge flow. |
| src/coreclr/nativeaot/Runtime/ObjectLayout.h | Adds BIT_SBLK_BRIDGE_PENDING for NativeAOT parity. |
| src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp | Implements pending-bit check/clear for NativeAOT Java bridge flow. |
| src/coreclr/gc/objecthandle.cpp | Records pending handle cells and notes the active-bridge recomputation race (FIXME). |
| src/coreclr/gc/gcbridge.h | Exposes pending-handle tracking APIs. |
| src/coreclr/gc/gcbridge.cpp | Implements pending-handle tracking and sets BIT_SBLK_BRIDGE_PENDING on candidates. |
Suppressed comments (2)
src/coreclr/vm/interoplibinterface_java.cpp:127
- When g_GCBridgeActive is already true, this early-return path releases args but never clears BIT_SBLK_BRIDGE_PENDING on the objects just marked in ProcessBridgeObjects for this GC. That can leave stale pending bits behind, causing future weak-handle checks to spuriously block (especially on subsequent bridge-active cycles). Clear the pending bits for the current pending handle set before returning.
if (g_GCBridgeActive)
{
// FIXME: This should become unreachable once bridge graph recomputation is skipped while active.
// Release the memory allocated since the GCBridge
// is already running and we're not passing them to it.
ReleaseGCBridgeArgumentsWorker(args);
return;
src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp:77
- When g_GCBridgeActive is already true, this early-return path releases args but never clears BIT_SBLK_BRIDGE_PENDING on the objects just marked in ProcessBridgeObjects for this GC. That can leave stale pending bits behind, causing future weak-handle checks to spuriously block. Clear the pending bits for the current pending handle set before returning.
if (g_GCBridgeActive)
{
// FIXME: This should become unreachable once bridge graph recomputation is skipped while active.
// Release the memory allocated since the GCBridge
// is already running and we're not passing them to it.
ReleaseGCBridgeArgumentsWorker(args);
return;
}
I can't comment on copilot's conclusion on its own created benchmark (which also doesn't reflect gc behavior for user app, where GC is triggered once every couple of seconds, both in their repro and in the actual game where Rolf tested - https://gist.github.com/rolfbjarne/5fd7ac636132718170ea30dc1be2452c. The copilot comment reports collection rates of 40 per second, which is absurd in my opinion, especially for a game). I tested on the exact sample provided by the customer (https://github.com/hyvanmielenpelit/Net11FPSBenchmark) where, following each gc, there were waits on the UI thread of ~30ms. The sample extracted logic from their actual game. While doing other investigation, starting from the maui sample, I simply triggered GCs directly from the button callback which resulted in waits as well. Aside from users simply using weak reference that they shouldn't expect to have significant overhead, the C#-Java interop layer is filled with uses of weak reference checks for bridge objects (so any native event that needs to bubble up into C# would be blocking unnecessarily on the java gc finish and there are probably dozens of other scenarios). I'm also putting into perpsective that the change is rather simplistic and is a nice to have optimization regardless. |
I think it is ok. It should be under |
|
Correction — that "ruled out" line is mine and it's wrong. I've edited the original comment on #131370. The experiment behind it compared a weak-reference arm against a strong-reference control and found Symbolized off-CPU profiling says the opposite. Ablating it confirms the size. A prototype of this same fix moves the probe 42.7 → 49.3 fps and Two caveats:
Details and the raw runs: https://github.com/steveisok/android-gcbridge-investigation Note This analysis was generated with GitHub Copilot. |
|
This fix will help, but the majority of the problem will remain in the cost of the promotion of these objects. |
7b98ec3 to
00a176a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/coreclr/vm/interoplibinterface_java.cpp:124
TriggerClientBridgeProcessingrelies on_ASSERTE(!g_GCBridgeActive)but has no retail guard. If this invariant is violated in a non-Debug build (e.g., redundant/overlapping bridge triggering), the code will proceed and can corrupt the pending-bridge bookkeeping and/or double-trigger client processing. The previous implementation handled this safely by releasingargsand returning.
size_t pendingBridgeHandleCount;
uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount);
_ASSERTE(!g_GCBridgeActive);
bool gcBridgeTriggered = JavaNative::TriggerClientBridgeProcessing(args);
src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp:74
TriggerClientBridgeProcessinguses_ASSERTE(!g_GCBridgeActive)as the only protection against overlapping bridge triggers. In retail builds this becomes a no-op; if the invariant is ever violated, the function will proceed and can corrupt pending-handle state or trigger client bridge processing twice. A defensive runtime check (as existed before) would make this robust.
size_t pendingBridgeHandleCount;
uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount);
_ASSERTE(!g_GCBridgeActive);
src/coreclr/gc/objecthandle.cpp:1533
- The comment above the
HndScanHandlesForGCcall no longer matches the newlp2usage (it is now a boolean indicating whether to record pending bridge handles, not a pointer for promotion data). This is likely to mislead future changes to the scanning callback contract.
// or have a local var for bridgeObjectsToPromote/size (instead of NULL) that's passed in as lp2
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/coreclr/vm/marshalnative.cpp:402
- The comment above this fast path is now misleading: this method no longer waits for bridge processing to finish, it only returns false for handles whose target is currently pending bridge processing. Please update the comment (and fix the typo) to match the new behavior, otherwise future readers may assume this blocks or fully synchronizes with the bridge.
FCIMPL2(FC_BOOL_RET, MarshalNative::GCHandleInternalTryGetBridgeWait, OBJECTHANDLE handle, Object **pObjResult)
{
FCALL_CONTRACT;
if (!Interop::TryGetObjectFromHandleWithoutBridgeWait(handle, pObjResult))
src/coreclr/vm/interoplibinterface_java.cpp:127
- Minor grammar: “wasn't trigger” should be “wasn't triggered”.
if (!gcBridgeTriggered)
{
// Release the memory allocated since the GCBridge
// wasn't trigger for some reason.
ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount);
2b5a92a to
fa2867f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/coreclr/vm/interoplibinterface_java.cpp:82
- TryGetObjectFromHandleWithoutBridgeWait declares
resultas an_Out_parameter but returnsfalsewithout writing*result. Even though current callers likely ignore the value onfalse, this violates the contract and can lead to accidental use of an uninitialized out value by future callers.
Object* object = OBJECTREFToObject(ObjectFromHandle(handle));
if (g_GCBridgeActive && object != nullptr &&
(object->GetHeader()->GetBits() & BIT_SBLK_BRIDGE_PENDING) != 0)
{
return false;
}
|
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. |
|
This addresses an issue reported on .net11 preview, as a regression from .net10 mono. Ideally we would get this in by Friday, in time for RC1 snap. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/coreclr/gc/gcbridge.cpp:1319
ProcessBridgeObjectssetsBIT_SBLK_BRIDGE_PENDINGon all registered bridge objects even ifBuildSccCallbackData()fails to allocate and returnsnullptr. In that case, the caller skips triggering client bridge processing, so the pending bits may remain set until the objects die, and a later bridge cycle could spuriously treat them as “pending” and force weak-ref waits.
Consider only setting the pending bit when args != NULL (or clearing the bits on the failure path).
for (int i = 0; i < DynPtrArraySize(&g_registeredBridges); i++)
{
Object* object = (Object*)DynPtrArrayGet(&g_registeredBridges, i);
object->GetHeader()->SetBit(BIT_SBLK_BRIDGE_PENDING);
}
Adds the A/B harness and results for dotnet/runtime#131952, the upstream implementation of the precise weak-reference wait prototyped here. The two arms differ only by that PR, which branched directly off the merge commit of #131764, and share one System.Private.CoreLib.dll because the PR changes no managed code. It reproduces the prototype (42.8 -> 49.9 fps at 1200 peers/frame) but recovers only ~31% of lost frame time at nodes=300, the operating point that matches the shape reported in #131370 -- so the wait accounts for less of the damage as the peer rate falls toward the realistic regime, not more. Also records the measurement trap this cost: an incremental app build does not re-copy a changed libcoreclr.so out of the runtime pack, so both arms silently run identical binaries and the result looks like a change that did nothing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 91dd8b98-73db-41b0-8d8c-9e1c21b53f5f
|
Any chance for a review to get this into rc1 ? |
MAUI startup A/B on Samsung A16I tested this change against the exact runtime source used by the installed Build
The two APKs were cloned from one base APK and re-signed after replacing Measurement
Results
Bridge confirmationA separate diagnostic startup with GC logging enabled recorded:
That is an approximately 42 ms accepted bridge round during startup. The ConclusionOn this peer-heavy MAUI sample, selective weak-reference waiting produces a These local CoreCLR builds do not have the official runtime pack's PGO/BOLT |
Bridge objects live in 2 worlds, .net and java so a .net bridge object has a correpsonding java peer. Collection of these objects is triggered by .net. When the .net peer is eligible for collection we build some graph over the set of dead objects and pass it over to java. Java triggers its own collection, collecting the java peers if they are dead as well. .NET android reports which bridge objects died on the java side so we can drop the gchandles for them. This will finally allow .net peers to die in the following collection (since they had to be promoted, given we don't know yet if java peers need to keep them alive or not). Currently obtaining the target of a weak ref blocks until the bridge processing is fully completed. This is the case also on mono and prevents 2 issues: - normal c# code checks a weak ref for some object. This can't immediately return correct information. If it returns true, the object gets resurrected and we can end up with a ref to a bridge objects that no longer has a java peer. If it returns false then that can be false as well if the object remains alive. - java code could call into managed, inserting a reference to a C# peer and afterward it could drop its own java peer. If this happens while C# gc ran but the java gc is still yet to start, both GC would see their peer as dead, even though it is alive. The .NET android interop obtains the C# peer ref also via weak reference, so this safely synchronizes with bridge processing. This PR keeps the weak reference wait only for bridge objects that are currently processed. For a weak ref target we need to determine whether the underlying object is pending bridge processing which is awkward to do efficiently because we would need to iterate over a set of handles or implement a lookup from obj address to associated cross reference handle. It turns out there is a free bit in the object header that we could use for this purpose. FIXME this has a race with redudndant bridge processing, because a new collection would dirty our g_registeredBridgeHandles.
… and clearing of pending bit The bit clearing already had memory ordering since it was done via InterlockedAnd. For the read we add GetBitsAcquire which does an acquire load, preserving the ordering on the reader side.
1db650f to
68531b9
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate issues remain in pending-bit cleanup and GC interface versioning, with requested regression coverage and documentation updates.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
|
This is a critical hotfix for Android, and I'd like to ask for it to be backported to release/11.0 rather than waiting for .NET 12. I ship a MAUI app on Android (CoreCLR, .NET 10), and this is my number 1 source of ANRs in production since upgrading to CoreCLR about a month ago. Sentry ANR reports consistently show the main thread in pthread_cond_wait under GCHandle_InternalGetBridgeWait while the bridge thread is in java.lang.Runtime.gc(); one session logged 895 ART collections. I found and fixed a peer leak of my own (window insets re-dispatched on every scroll step), which made it worse, but even after that fix my Play user-perceived ANR rate is still around 2.5–4.5% against the 0.47% bad-behaviour threshold, and may drop further once all users have updated. My only mitigation is DOTNET_GCgen0size=32 MB (17 → 2 bridge rounds in a 45 s launch), and it makes each remaining stall longer whenever peers churn. As @steveisok's profiling above showed, the waits come from the interop layer's own peer bookkeeping, so any Android app with Java peers hits this, whether or not it touches WeakReference itself. The more Java peers an app creates while the UI is active, the worse it gets. Scrolling a CollectionView or ListView does exactly that (see also dotnet/android#11567, explicit Java GCs during scrolling), so most apps will feel it. And the original report is from a game, with ~30 ms UI-thread waits after every GC. With CoreCLR becoming the default Android runtime in .NET 11, every app that upgrades from Mono will silently pick up this regression: jank and ANRs that weren't there on Mono, with nothing in their own code to explain them. Without a backport they'll live with it until .NET 12 next November. The fix was aimed at the RC1 snap and appears to have missed it only because of review timing. Ideally this would ship in .NET 11 GA. If that's no longer possible, could it be considered for release/11.0 servicing (11.0.1/11.0.2)? |
|
@radimitrov I'm somewhat skeptical about your conclusion for 2 reasons:
My worry is that the main reason for what you are experiencing is something else and backporting this would not necessarily fix your issue. I would be interested in doing some GC profiling / comparison with Mono if you are open to share a repro. (Showcasing significantly worse perf compared with Mono is still valuable, ANR crash is not mandatory). Note you can reach me privately on vlbrez@microsoft.com. |
|
@BrzVlad In my case it was an error on my part causing thousands of redundant Java peers every time a CollectionView was scrolled. The bridge GC of these objects was causing a UI freeze. Fixing the thousands of reduntant Java peers caused from the edge to edge code seems to have mitigated a lot of the issue, though I can't be sure by how much yet, until some more data is gathered over the next week at least. I think the ANR itself was almost exclusevely on lower end devices after too much background thread work piled up. It is a heavy IDE app, no way around most of the memory pressure. So yes, even I was at best 80% certain of my conclusion when I posted this, but I thought it is better to do it now rather than potentially miss a release window SR1 and SR2. I still think this fix making it into .NET 11 is a good idea, even if it does nothing to help in my case. I have added additional diagnostics to determine any other ANR and memory issues. I will try to put together an isolated reproduction to confirm. |
|
@BrzVlad Thanks, you were right to push back. Here's an isolated repro: https://github.com/radimitrov/android-coreclr-gcbridge-repro, the results are there as an MD file. It is a plain .NET for Android app (no MAUI) plus one script (
On your two points
1. Many small rounds (background allocation): this PR removes most of the UI cost
Same GC work (same managed GC counts, same rounds, same Java GC time), about 24× less lost UI time. With light background work, the worst freeze drops from 155 ms to 68 ms. A large win at no GC cost, so I'd still ask for it in .NET 11 servicing. 2. Large rounds: freezes remain, and this PR doesn't remove them
Rounds here carry roughly 34k (11) to 50k (12) dead peers, against a similar ~50k on Mono. About 90% of each CoreCLR freeze comes after
What I can't explain yet: with #131952 the UI should only wait for objects pending in the round, and in Separately, for .NET 10 CoreCLR uses the 256 KB gen0 floor on Android: release/10.0 defines None of this reached ANR length (5 s) on this device, but the large-round freezes grow faster than linearly with dead peers, and a low-end phone does the same work several times slower. Happy to run anything else that helps. In truth .NET 11 RC1 performs better than .NET 10 so the real world performance will probably already be better even without the fix, also unless I am mistaken there is still work actively being done on the GC area for Android. So I will leave it entirely to your judgement if this PR should make it into a .NET 11 release. Results
|

Bridge objects live in 2 worlds, .net and java so a .net bridge object has a correpsonding java peer. Collection of these objects is triggered by .net. When the .net peer is eligible for collection we build some graph over the set of dead objects and pass it over to java. Java triggers its own collection, collecting the java peers if they are dead as well. .NET android reports which bridge objects died on the java side so we can drop the gchandles for them. This will finally allow .net peers to die in the following collection (since they had to be promoted, given we don't know yet if java peers need to keep them alive or not).
Currently obtaining the target of a weak ref blocks until the bridge processing is fully completed. This is the case also on mono and prevents 2 issues:
This PR keeps the weak reference wait only for bridge objects that are currently processed. For a weak ref target we need to determine whether the underlying object is pending bridge processing which is awkward to do efficiently because we would need to iterate over a set of handles or implement a lookup from obj address to associated cross reference handle. It turns out there is a free bit in the object header that we could use for this purpose.
Addresses #131370