Skip to content

[Java.Interop] Lazily allocate peer member caches - #12701

Merged
simonrozsival merged 8 commits into
mainfrom
simonrozsival-lazy-jni-instance-methods
Sep 8, 2026
Merged

[Java.Interop] Lazily allocate peer member caches#12701
simonrozsival merged 8 commits into
mainfrom
simonrozsival-lazy-jni-instance-methods

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

  • allocate instance/static method and field caches only on first use
  • allocate the managed-subclass constructor cache only when a derived constructor path is needed
  • share lock-free cache publication through a generic GetOrCreate<TKey, TValue>() helper
  • atomically detach and clear allocated caches during disposal
  • cover deferred allocation and concurrent first use

Analysis

Across Mono.Android for API 37, the types in our bindings have the following numbers of methods and fields:

Category 0 1 2–4 >4 Mean
Instance methods + constructors 5.9% 15.2% 31.9% 47.1% 8.09
Instance fields 95.5% 1.2% 2.2% 1.1% 0.18
Static methods 82.6% 5.4% 7.8% 4.2% 0.95
Static fields 81.6% 13.6% 3.1% 1.7% 0.61

This means that for most types, we should not be allocating a heavy ConcurrentDictionary for instance fields, static methods, and static fields. It's also possible that for some binding types we won't call even a single of their methods even though their static constructor runs. It makes sense to reduce allocations and reduce gc pressure here.

Results

A Release trimmed CoreCLR arm64 app created from dotnet new maui --sample-content was measured through first window focus on a Samsung A16.

Current main eagerly allocates approximately 1,347 peer cache dictionaries (~274 KiB at the measured 208 B per empty dictionary). With this change, first launch allocated 273 dictionaries (~55 KiB), avoiding about 1,074 dictionary allocations / 218 KiB gross managed allocation.

Repeated process-cold launches allocated 271–273 dictionaries (average 272):

Cache Current main (eager) This PR (lazy)
Instance methods 265–267 198–199
Subclass constructors 265–267 33
Instance fields 264–266 10
Static methods 264–266 22
Static fields 264–266 8–9
Total 1,322–1,332 271–273

The eager counts differ by one because one managed-subclass constructor path creates an additional JniInstanceMethods helper; only primary JniPeerMembers instances own the field/static caches.

The earlier instance-method-only version avoided about 67 dictionaries (~14 KiB); most savings come from deferring the other four caches.

Startup timing

A final all-cache A/B used 64 paired process-cold launches in four package-ID crossover blocks on the Samsung A16 (Release trimmed CoreCLR arm64, speed-compiled packages, animations disabled, alternating order, 27.6–28.0 °C):

Variant Mean Median
Current main (47eabaf) 2,232.13 ms 2,220.5 ms
This PR 2,220.58 ms 2,212.5 ms

The paired delta was −11.55 ms (−0.52%), favoring this PR, but was not statistically significant: 95% t interval [−28.06, +4.97] ms, bootstrap interval [−27.98, +4.16] ms, with 36/64 wins and one tie.

The honest conclusion is no measurable startup regression or improvement. Reduced managed allocation and retained heap are the demonstrated benefits.

Concurrent Dispose() and cache access remains unsupported, as before. Interlocked.Exchange() atomically detaches each cache visible to disposal but is not a disposed-state guard for stale references.

Retained startup heap

Two startup GC dumps per revision were collected after first window focus from the same Release trimmed CoreCLR arm64 dotnet new maui --sample-content app on Samsung A16. dotnet-gcdump collect induced a Gen2 GC, so values are live retained objects.

Run Current main This PR Delta
Clean first launch 4,568,463 B / 65,446 objects 4,390,711 B / 61,096 objects -177,752 B / -4,350 objects
Process-cold repeat 4,487,244 B / 64,667 objects 4,312,748 B / 60,382 objects -174,496 B / -4,285 objects
Mean 4,527,854 B / 65,057 objects 4,351,730 B / 60,739 objects -176,124 B (-3.89%) / -4,318 (-6.64%)

The targeted cache dictionary counts were 1,141/1,126 on current main versus 276/274 with this PR, avoiding 865/852 retained dictionaries. The entire retained ConcurrentDictionary delta is exactly these Java.Interop caches.

Useful cache contents were unchanged: JniMethodInfo counts were 818/815 for both revisions, JniFieldInfo was 36 for both, and method, field, and subclass node counts were unchanged. The dumps show removal of empty containers only.

Tests

  • Java.Interop Debug build: 0 warnings, 0 errors
  • focused JniPeerMembersTests: 12 passed, 1 skipped
  • full Java.Interop-Tests: 711 passed, 6 skipped, 0 failed

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 7, 2026 08:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The new concurrency test should ensure JniPeerMembers.Dispose(members) runs via try/finally to avoid leaking state when assertions fail.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity external/​Java.Interop/​tests/​Java.Interop-Tests/​Java.Interop/​JniPeerMembersTests.cs⚠️ Resource managementmembers is disposed only at the end of the test, so any assertion…
What changed in this PR

This PR updates Java.Interop’s JniPeerMembers.JniInstanceMethods to lazily allocate the per-type instance method cache on first use, using Volatile.Read + Interlocked.CompareExchange for safe publication, and adds unit test coverage for deferred allocation and concurrent first access.

Changes:

  • Make JniInstanceMethods.InstanceMethods lazily allocated instead of eagerly created.
  • Publish the cache safely using Volatile.Read and Interlocked.CompareExchange, and detach it atomically during disposal.
  • Add tests verifying deferred allocation and concurrent first-use behavior.
File Description
external/​Java.Interop/​src/​Java.Interop/​Java.Interop/​JniPeerMembers.JniInstanceMethods.cs Lazily allocates and safely publishes the instance-method cache; atomically detaches it during disposal.
external/​Java.Interop/​tests/​Java.Interop-Tests/​Java.Interop/​JniPeerMembersTests.cs Updates existing expectations for lazy allocation and adds a concurrent first-use test.

simonrozsival and others added 2 commits September 7, 2026 10:34
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival changed the title [Java.Interop] Lazily allocate instance method cache [Java.Interop] Lazily allocate peer member caches Sep 7, 2026
simonrozsival and others added 5 commits September 7, 2026 10:49
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>
@simonrozsival
simonrozsival enabled auto-merge (squash) September 7, 2026 19:36
@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Sep 7, 2026
@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12701

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM

No blocking correctness, safety, interop, or performance issues found. The lock-free publication preserves the existing ordinal key semantics, disposal behavior remains intentionally unsupported under concurrent access, and all 44 reported checks are green.

Findings: 0 errors · 0 warnings · 1 suggestion

The allocation measurements and focused cache coverage are strong. I left one inline suggestion to make the concurrency regression test deterministic.

Generated by Android PR Reviewer for #12701 · gpt56 · 128.9 AIC · ⌖ 19 AIC · ⊞ 25.7K
Comment /review to run again

var constructors = new JniMethodInfo [16];

Assert.IsNull (GetInstanceMethods (methods));
Parallel.For (0, constructors.Length, i => constructors [i] = methods.GetConstructor ("()V"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 💡 Testing — Please synchronize the workers before they call GetConstructor() (for example with a Barrier) so they are guaranteed to contend on the initial null cache. Parallel.For may execute these short iterations serially or only after the first iteration has already published the dictionary, allowing a broken non-atomic initializer to pass nondeterministically.

Rule: Deterministic concurrency coverage

@simonrozsival
simonrozsival merged commit c081097 into main Sep 8, 2026
44 checks passed
@simonrozsival
simonrozsival deleted the simonrozsival-lazy-jni-instance-methods branch September 8, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants