[CoreCLR/NativeAOT] Use a no-GC region during Android startup - #12782
simonrozsival wants to merge 10 commits into
Conversation
Start a bounded 24 MiB no-GC region during CoreCLR initialization and end it after Activity.ReportFullyDrawn. Preserve the generated binding contract manually and use a timeout plus collection tracking for safe fallback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Unresolved moderate API, dispatch, and no-GC ownership issues remain.
Review tier: Lite
Findings: None
What changed in this PR
Adds a CoreCLR-only Android startup no-GC region, ending after ReportFullyDrawn() or a timeout.
Changes:
- Adds bounded no-GC lifecycle management and startup initialization.
- Provides a manual
ReportFullyDrawn()JNI binding. - Updates metadata, project files, and lifecycle tests.
| File | Summary |
|---|---|
tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj |
Registers the new test coverage. |
tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/StartupNoGCRegionTest.cs |
Tests lifecycle, timeout, collection, and registration behavior. |
src/Mono.Android/Mono.Android.csproj |
Includes the startup region implementation. |
src/Mono.Android/metadata |
Removes the generated ReportFullyDrawn binding. |
src/Mono.Android/Android.Runtime/StartupNoGCRegion.cs |
Implements bounded startup region management; a concurrency race remains. |
src/Mono.Android/Android.Runtime/JNIEnvInit.cs |
Starts the region during CoreCLR initialization. |
src/Mono.Android/Android.App/Activity.cs |
Adds the manual binding and end hook; API metadata, virtual dispatch, and integration coverage issues remain. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Reject
Findings: 2 errors, 0 warnings, 0 suggestions.
The measured startup improvement is promising, and the manual Activity.ReportFullyDrawn() binding preserves the public/JNI shape. However, the implementation unconditionally takes ownership of the process-wide no-GC facility and its collection-count heuristic cannot guarantee that it will not end a customer replacement region. Both compatibility hazards need resolution before merge.
CI: Azure DevOps build 1595487 was still in progress when reviewed. The completed Android Tools/Java.Interop lanes and CLA check were green; no failures were reported yet.
Generated by Android PR Reviewer for #12782 · copilot · gpt56 · 186.2 AIC · ⌖ 11.3 AIC · ⊞ 21K
Comment /review to run again
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
simonrozsival
changed the title
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
|
There was a problem hiding this comment.
❌ Reject
Findings: 2 errors, 0 warnings, 0 suggestions.
The startup measurements are encouraging, and the manual Activity.ReportFullyDrawn() binding preserves the existing public/JNI shape. However, the current revision reintroduces two process-wide GC ownership hazards: it provides no compatibility escape hatch for applications that manage their own no-GC region, and a delayed completion can terminate a replacement region after ownership has been lost.
CI: Azure DevOps build 1595775 is still in progress. At review time, 17 checks had passed, 25 were running, 1 was queued, and dotnet-android (MAUI Tests MAUI Integration) was failing. The available integration could not retrieve the Azure failure details, so I could not determine whether that lane is related to this change.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
azcliprod.blob.core.windows.net
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
Generated by Android PR Reviewer for #12782 · copilot · gpt56 · 140.1 AIC · ⌖ 8.65 AIC · ⊞ 21.8K
Comment /review to run again
Guard startup and completion callers so disabled no-GC support can be trimmed away. Cover the default and explicit switch settings in linker tests, and explain why ReportFullyDrawn remains manually bound. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 0 errors, 1 warning, 0 suggestions.
The runtime feature switch provides the requested private escape hatch, and the manual Activity.ReportFullyDrawn() implementation preserves the public/JNI binding shape. The remaining gap is regression coverage: the added linker test proves trimming behavior only, while the process-wide no-GC lifecycle, timeout, failure, and concurrency paths are untested.
CI: Azure DevOps build 1602176 was still in progress when reviewed: 7 checks passed, 36 were running, 1 was queued, and no failures were reported.
Generated by Android PR Reviewer for #12782 · copilot · gpt56 · 139.8 AIC · ⌖ 9.83 AIC · ⊞ 17.6K
Comment /review to run again
| using var b = CreateApkBuilder (); | ||
| Assert.IsTrue (b.Build (proj), "Build should have succeeded."); | ||
| using var assembly = AssemblyDefinition.ReadAssembly (BuildTest.GetLinkedPath (b, true, "Mono.Android.dll")); | ||
| var type = assembly.MainModule.GetType ("Android.Runtime.StartupNoGCRegion"); |
There was a problem hiding this comment.
🤖 ReportFullyDrawn() or the timeout from ending the region, calls EndNoGCRegion() after a failed start, or loses the one-shot/concurrent guarantee would still pass. Please add focused runtime/device coverage for successful start/end, start failure, repeated/concurrent completion, and the fallback path (the earlier lifecycle test seam covered these cases).
Rule: Test edge cases
|
|
||
| sealed class StartupNoGCRegion | ||
| { | ||
| const long Budget = 24 * 1024 * 1024; |
There was a problem hiding this comment.
Does this work well on Android Arm32?
I would expect TryStartNoGCRegion is always going to fail with 24MB reservation on 32-bit platforms.
| bool started; | ||
| try { | ||
| started = GC.TryStartNoGCRegion (Budget, disallowFullBlockingGC: true); | ||
| } catch (InvalidOperationException) { |
There was a problem hiding this comment.
You may want to catch all exceptions.
TryStartNoGCRegion can also throw ArgumentOutOfRangeException when the Budget is too high for the current GC configuration (like on 32-bit platforms).
| // The binding generator has no hook for injecting the startup no-GC cleanup into this method. | ||
| [SupportedOSPlatform ("android19.0")] | ||
| [Register ("reportFullyDrawn", "()V", "GetReportFullyDrawnHandler")] | ||
| public virtual unsafe void ReportFullyDrawn () |
There was a problem hiding this comment.
I still think this seems odd, do we already bind this? We document it somehow?!?
If we don't bind it, can we figure out why it's not bound already? Android docs here:
Then we can decide if a manual binding is appropriate, or if we need to just fix some binding bug or metadata transform instead.
Rationale
CoreCLR and NativeAOT Android startup can incur managed GC work before the first frame is fully drawn. Reserve a short startup allocation budget so this work is deferred until the app reports that startup is complete.
Implementation
disallowFullBlockingGC: true.Activity.ReportFullyDrawn()completes.Activitypartial class.ReportFullyDrawn()with a 10-second one-shot timer.Benchmark evidence
Samsung A16, five matched launches using an older CoreCLR trimmable configuration:
The startup request itself performed a near-empty preparation collection at about 120 KiB allocated. The Android GC bridge's explicit ART collection moved until after fully drawn.
The final complete-MIBC/RCPC2-safe configuration used the same complete trimmable MIBC for both variants (
eefb408c483e571af59d04074e1f4402b2d74eaade4d89cec6c329066256755b, 244,583 bytes). An earlier sequential comparison measured 1,177.5 ms without startup no-GC and 1,229 ms with it, but an alternating matched rerun did not reproduce that regression:The no-GC region started and ended successfully in all ten side-by-side launches. The bootstrap 95% interval for the side-by-side median difference was -51 to +3 ms, so the rerun rules out the previously reported regression but supports only a small, not yet statistically definitive improvement in the final optimized configuration.
Risks and fallback
The region is process-wide by design and intentionally short-lived. It ends on the first managed fully-drawn report or after 10 seconds. The runtime automatically leaves the region if the 24 MiB budget is exhausted or a collection is induced. All start and end paths are best effort and one-shot.
Validation
Mono.AndroidAPI 37 build passed.Microsoft.Android.Runtime.NativeAOTbuild passed.ReportFullyDrawnmethod is removed in favor of the manual binding.Activity.ReportFullyDrawn()remains public, virtual, supported from Android 19, and registered asreportFullyDrawn,()V,GetReportFullyDrawnHandler.