Skip to content

Commit 060bda7

Browse files
steveisokCopilot
authored andcommitted
Avoid redundant Java GC bridge graph computation (dotnet#131764)
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
1 parent 0e3a635 commit 060bda7

13 files changed

Lines changed: 86 additions & 5 deletions

src/coreclr/gc/env/gcenv.ee.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ class GCToEEInterface
4141

4242
static void TriggerClientBridgeProcessing(MarkCrossReferencesArgs* args);
4343

44+
static bool IsClientBridgeProcessingActive();
45+
4446
// Sync block cache management
4547
static void SyncBlockCacheWeakPtrScan(HANDLESCANPROC scanProc, uintptr_t lp1, uintptr_t lp2);
4648
static void SyncBlockCacheDemote(int max_gen);

src/coreclr/gc/env/gctoeeinterface.standalone.inl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ namespace standalone
5454
return ::GCToEEInterface::TriggerClientBridgeProcessing(args);
5555
}
5656

57+
bool IsClientBridgeProcessingActive()
58+
{
59+
return ::GCToEEInterface::IsClientBridgeProcessingActive();
60+
}
61+
5762
void SyncBlockCacheWeakPtrScan(HANDLESCANPROC scanProc, uintptr_t lp1, uintptr_t lp2)
5863
{
5964
::GCToEEInterface::SyncBlockCacheWeakPtrScan(scanProc, lp1, lp2);

src/coreclr/gc/gcbridge.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,6 +1111,13 @@ uint8_t** GetRegisteredBridges(size_t* pNumBridges)
11111111
return (uint8_t**)g_registeredBridges.data;
11121112
}
11131113

1114+
bool ShouldProcessBridgeObjects()
1115+
{
1116+
// The client discards any set of cross references handed to it while it is still
1117+
// processing a previous one, so computing it would be pure waste.
1118+
return !GCToEEInterface::IsClientBridgeProcessingActive();
1119+
}
1120+
11141121
static bool TarjanSccAlgorithm()
11151122
{
11161123
int i;

src/coreclr/gc/gcbridge.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
void BridgeResetData();
1313
MarkCrossReferencesArgs* ProcessBridgeObjects();
1414

15+
// Decides whether this collection should hand a fresh set of cross references to the client.
16+
// Returns false when the client is still processing a previous set, since the new one would
17+
// just be discarded.
18+
bool ShouldProcessBridgeObjects();
19+
1520
void RegisterBridgeObject(Object *object, uintptr_t context);
1621
uint8_t** GetRegisteredBridges(size_t *pNumBridges);
1722

src/coreclr/gc/gcenv.ee.standalone.inl

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,17 @@ inline void GCToEEInterface::TriggerClientBridgeProcessing(MarkCrossReferencesAr
8181
}
8282
}
8383

84+
inline bool GCToEEInterface::IsClientBridgeProcessingActive()
85+
{
86+
assert(g_theGCToCLR != nullptr);
87+
if (g_runtimeSupportedVersion.MajorVersion >= 5)
88+
{
89+
return g_theGCToCLR->IsClientBridgeProcessingActive();
90+
}
91+
92+
return false;
93+
}
94+
8495
inline void GCToEEInterface::SyncBlockCacheWeakPtrScan(HANDLESCANPROC scanProc, uintptr_t lp1, uintptr_t lp2)
8596
{
8697
assert(g_theGCToCLR != nullptr);

src/coreclr/gc/gcinterface.ee.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,14 @@ class IGCToCLR {
467467

468468
virtual
469469
void TriggerClientBridgeProcessing(MarkCrossReferencesArgs* args) PURE_VIRTUAL
470+
471+
// The following method is available only with EE_INTERFACE_MAJOR_VERSION >= 5
472+
473+
// Returns true when the client is still processing cross references handed to it by a
474+
// previous call to TriggerClientBridgeProcessing. While that is the case any new set of
475+
// cross references would be discarded by the client, so the GC can skip computing it.
476+
virtual
477+
bool IsClientBridgeProcessingActive() PURE_VIRTUAL
470478
};
471479

472480
#endif // _GCINTERFACE_EE_H_

src/coreclr/gc/gcinterface.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
// The major version of the IGCToCLR interface. Breaking changes to this interface
1717
// require bumps in the major version number.
18-
#define EE_INTERFACE_MAJOR_VERSION 4
18+
#define EE_INTERFACE_MAJOR_VERSION 5
1919

2020
struct ScanContext;
2121
struct gc_alloc_context;

src/coreclr/gc/objecthandle.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,13 +1535,19 @@ uint8_t** Ref_ScanBridgeObjects(uint32_t condemned, uint32_t maxgen, ScanContext
15351535
}
15361536

15371537
// The callee here will free the allocated memory.
1538-
MarkCrossReferencesArgs *args = ProcessBridgeObjects();
1539-
1540-
if (args != NULL)
1538+
if (ShouldProcessBridgeObjects())
15411539
{
1542-
GCToEEInterface::TriggerClientBridgeProcessing(args);
1540+
MarkCrossReferencesArgs *args = ProcessBridgeObjects();
1541+
1542+
if (args != NULL)
1543+
{
1544+
GCToEEInterface::TriggerClientBridgeProcessing(args);
1545+
}
15431546
}
15441547

1548+
// Every registered bridge object is promoted whether or not the cross references were
1549+
// computed above, so skipping the work while the client is busy only delays reporting a
1550+
// dead peer, it never collects one early.
15451551
return GetRegisteredBridges(numObjs);
15461552
}
15471553
#endif // FEATURE_JAVAMARSHAL

src/coreclr/gc/sample/gcenv.ee.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ void GCToEEInterface::TriggerClientBridgeProcessing(MarkCrossReferencesArgs* arg
168168
{
169169
}
170170

171+
bool GCToEEInterface::IsClientBridgeProcessingActive()
172+
{
173+
return false;
174+
}
175+
171176
bool GCToEEInterface::IsPreemptiveGCDisabled()
172177
{
173178
Thread* pThread = ::GetThread();

src/coreclr/nativeaot/Runtime/gcenv.ee.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,4 +826,13 @@ void GCToEEInterface::TriggerClientBridgeProcessing(MarkCrossReferencesArgs* arg
826826
#endif
827827
}
828828

829+
bool GCToEEInterface::IsClientBridgeProcessingActive()
830+
{
831+
#ifdef FEATURE_JAVAMARSHAL
832+
return JavaMarshalNative::IsGCBridgeActive();
833+
#else
834+
return false;
835+
#endif
836+
}
837+
829838
#endif // !DACCESS_COMPILE

0 commit comments

Comments
 (0)