[CoreCLR/NativeAOT] Use a no-GC region during Android startup - #12782
simonrozsival wants to merge 9 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. |
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>
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
| timer?.Dispose (); | ||
|
|
||
| try { | ||
| GC.EndNoGCRegion (); |
There was a problem hiding this comment.
🤖 ❌ Resource management — GC.EndNoGCRegion() has no ownership token. If this region is ended implicitly by budget exhaustion or an induced collection, application code can start a replacement region before this timeout/ReportFullyDrawn() path runs; this call will then terminate the application-owned region. The current lock only serializes this helper and cannot protect against external GC callers. Please use a design that cannot end a replacement region (or add runtime support for conditional ownership), and restore deterministic coverage for this interleaving.
Rule: Thread safety of shared state
| throw new NotSupportedException ("Internal error: exactly one of RuntimeFeature.IsMonoRuntime or RuntimeFeature.IsCoreClrRuntime must be enabled."); | ||
| } | ||
|
|
||
| StartupNoGCRegion.Start (); |
There was a problem hiding this comment.
🤖 ❌ API design — This again unconditionally occupies the process-wide no-GC facility before RunStartupHooksIfNeeded(). A customer startup hook or early library that uses GC.TryStartNoGCRegion() will now fail until this region ends, and the opt-out introduced in the earlier revision has been removed. Please restore a documented runtime/MSBuild escape hatch (or make the optimization opt-in) so existing applications can preserve their prior GC behavior.
Rule: Behavior-safe defaults
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 CoreCLR trimmable launches:
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.
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.