From 5d91fc565342620991ac985040440d4e476dade5 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 13 Aug 2026 11:37:01 -0500 Subject: [PATCH] Use concurrent JNI peer caches Use ConcurrentDictionary with static GetOrAdd factories for the JniPeerMembers instance and static method and field caches, plus subclass constructor dispatch. Cached JNI member lookups no longer enter monitor locks, while existing remapping, fallback, exception, and lifetime behavior is preserved. Add an instrumentation-only Android BenchmarkDotNet app for measuring JNI member lookup and invocation changes on-device. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 45264ff0-e493-4402-a76f-576afd88ee70 --- Xamarin.Android-Tests.slnx | 1 + .../JniPeerMembers.JniInstanceFields.cs | 19 ++--- .../JniPeerMembers.JniInstanceMethods.cs | 51 +++--------- .../JniPeerMembers.JniStaticFields.cs | 19 ++--- .../JniPeerMembers.JniStaticMethods.cs | 25 ++---- .../Java.Interop/JniPeerMembersTests.cs | 6 +- .../Android.Benchmarks.csproj | 22 +++++ tests/Android.Benchmarks/AndroidManifest.xml | 4 + .../BenchmarkInstrumentation.cs | 83 +++++++++++++++++++ .../JniMethodInfoBenchmarks.cs | 42 ++++++++++ tests/Android.Benchmarks/README.md | 19 +++++ 11 files changed, 207 insertions(+), 84 deletions(-) create mode 100644 tests/Android.Benchmarks/Android.Benchmarks.csproj create mode 100644 tests/Android.Benchmarks/AndroidManifest.xml create mode 100644 tests/Android.Benchmarks/BenchmarkInstrumentation.cs create mode 100644 tests/Android.Benchmarks/JniMethodInfoBenchmarks.cs create mode 100644 tests/Android.Benchmarks/README.md diff --git a/Xamarin.Android-Tests.slnx b/Xamarin.Android-Tests.slnx index 18741c6dc9b..4bc3d8fd1e6 100644 --- a/Xamarin.Android-Tests.slnx +++ b/Xamarin.Android-Tests.slnx @@ -27,6 +27,7 @@ + diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs index e0a5ae40d87..43c33bdf95d 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs @@ -1,7 +1,7 @@ #nullable enable using System; -using System.Collections.Generic; +using System.Collections.Concurrent; namespace Java.Interop { @@ -15,7 +15,7 @@ internal JniInstanceFields (JniPeerMembers members) readonly JniPeerMembers Members; - Dictionary InstanceFields = new Dictionary(StringComparer.Ordinal); + readonly ConcurrentDictionary InstanceFields = new ConcurrentDictionary (1, 3, StringComparer.Ordinal); internal void Dispose () { @@ -24,16 +24,11 @@ internal void Dispose () public JniFieldInfo GetFieldInfo (string encodedMember) { - lock (InstanceFields) { - if (!InstanceFields.TryGetValue (encodedMember, out var f)) { - string field, signature; - JniPeerMembers.GetNameAndSignature (encodedMember, out field, out signature); - f = Members.JniPeerType.GetInstanceField (field, signature); - InstanceFields.Add (encodedMember, f); - } - return f; - } + return InstanceFields.GetOrAdd (encodedMember, static (member, fields) => { + string field, signature; + JniPeerMembers.GetNameAndSignature (member, out field, out signature); + return fields.Members.JniPeerType.GetInstanceField (field, signature); + }, this); } }} } - diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs index 6d8d14602ca..90ababdb74b 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs @@ -1,7 +1,7 @@ #nullable enable using System; -using System.Collections.Generic; +using System.Collections.Concurrent; namespace Java.Interop { @@ -39,8 +39,8 @@ internal JniType JniPeerType { readonly Type DeclaringType; - Dictionary InstanceMethods = new Dictionary(StringComparer.Ordinal); - Dictionary SubclassConstructors = new Dictionary (); + readonly ConcurrentDictionary InstanceMethods = new ConcurrentDictionary (1, 3, StringComparer.Ordinal); + readonly ConcurrentDictionary SubclassConstructors = new ConcurrentDictionary (1, 1); internal void Dispose () { @@ -58,13 +58,8 @@ public JniMethodInfo GetConstructor (string signature) { if (signature == null) throw new ArgumentNullException (nameof (signature)); - lock (InstanceMethods) { - if (!InstanceMethods.TryGetValue (signature, out var m)) { - m = JniPeerType.GetConstructor (signature); - InstanceMethods.Add (signature, m); - } - return m; - } + return InstanceMethods.GetOrAdd (signature, static (member, methods) => + methods.JniPeerType.GetConstructor (member), this); } internal JniInstanceMethods GetConstructorsForType (Type declaringType) @@ -72,13 +67,7 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) if (declaringType == DeclaringType) return this; - JniInstanceMethods? methods; - - lock (SubclassConstructors) { - if (SubclassConstructors.TryGetValue (declaringType, out methods)) - return methods; - } - // Init outside of `lock` in case we have recursive access: + // Initialize before publication in case construction recursively accesses this cache: // System.ArgumentException: An item with the same key has already been added. Key: Java.Interop.JavaProxyThrowable // at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) // at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value) @@ -100,32 +89,16 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) // at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 27 // at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77 // at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 146 - methods = new JniInstanceMethods (declaringType); - lock (SubclassConstructors) { - if (SubclassConstructors.TryGetValue (declaringType, out var m)) - return m; - SubclassConstructors.Add (declaringType, methods); - return methods; - } + return SubclassConstructors.GetOrAdd (declaringType, static type => new JniInstanceMethods (type)); } public JniMethodInfo GetMethodInfo (string encodedMember) { - lock (InstanceMethods) { - if (InstanceMethods.TryGetValue (encodedMember, out var m)) { - return m; - } - } - string method, signature; - JniPeerMembers.GetNameAndSignature (encodedMember, out method, out signature); - var info = GetMethodInfo (method, signature); - lock (InstanceMethods) { - if (InstanceMethods.TryGetValue (encodedMember, out var m)) { - return m; - } - InstanceMethods.Add (encodedMember, info); - } - return info; + return InstanceMethods.GetOrAdd (encodedMember, static (member, methods) => { + string method, signature; + JniPeerMembers.GetNameAndSignature (member, out method, out signature); + return methods.GetMethodInfo (method, signature); + }, this); } JniMethodInfo GetMethodInfo (string method, string signature) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs index 7fe131b6206..f0c490460ab 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs @@ -1,7 +1,7 @@ #nullable enable using System; -using System.Collections.Generic; +using System.Collections.Concurrent; namespace Java.Interop { @@ -15,19 +15,15 @@ internal JniStaticFields (JniPeerMembers members) readonly JniPeerMembers Members; - Dictionary StaticFields = new Dictionary(StringComparer.Ordinal); + readonly ConcurrentDictionary StaticFields = new ConcurrentDictionary (1, 3, StringComparer.Ordinal); public JniFieldInfo GetFieldInfo (string encodedMember) { - lock (StaticFields) { - if (!StaticFields.TryGetValue (encodedMember, out var f)) { - string field, signature; - JniPeerMembers.GetNameAndSignature (encodedMember, out field, out signature); - f = Members.JniPeerType.GetStaticField (field, signature); - StaticFields.Add (encodedMember, f); - } - return f; - } + return StaticFields.GetOrAdd (encodedMember, static (member, fields) => { + string field, signature; + JniPeerMembers.GetNameAndSignature (member, out field, out signature); + return fields.Members.JniPeerType.GetStaticField (field, signature); + }, this); } internal void Dispose () @@ -36,4 +32,3 @@ internal void Dispose () } }} } - diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs index 00b391bc112..379d6f21c52 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs @@ -1,7 +1,7 @@ #nullable enable using System; -using System.Collections.Generic; +using System.Collections.Concurrent; namespace Java.Interop { @@ -15,7 +15,7 @@ internal JniStaticMethods (JniPeerMembers members) internal readonly JniPeerMembers Members; - Dictionary StaticMethods = new Dictionary(StringComparer.Ordinal); + readonly ConcurrentDictionary StaticMethods = new ConcurrentDictionary (1, 3, StringComparer.Ordinal); internal void Dispose () { @@ -24,21 +24,11 @@ internal void Dispose () public JniMethodInfo GetMethodInfo (string encodedMember) { - lock (StaticMethods) { - if (StaticMethods.TryGetValue (encodedMember, out var m)) { - return m; - } - } - string method, signature; - JniPeerMembers.GetNameAndSignature (encodedMember, out method, out signature); - var info = GetMethodInfo (method, signature); - lock (StaticMethods) { - if (StaticMethods.TryGetValue (encodedMember, out var m)) { - return m; - } - StaticMethods.Add (encodedMember, info); - } - return info; + return StaticMethods.GetOrAdd (encodedMember, static (member, methods) => { + string method, signature; + JniPeerMembers.GetNameAndSignature (member, out method, out signature); + return methods.GetMethodInfo (method, signature); + }, this); } JniMethodInfo GetMethodInfo (string method, string signature) @@ -160,4 +150,3 @@ public unsafe JniObjectReference InvokeObjectMethod (string encodedMember, JniAr } }} } - diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs index b065dd8c14c..45b2a3cb6e1 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs @@ -1,5 +1,5 @@ using System; -using System.Collections.Generic; +using System.Collections.Concurrent; using System.Reflection; using Java.Interop; @@ -30,10 +30,10 @@ public void VirtualInvokeOnBaseInvokesMostDerivedJavaMethod () } } - static Dictionary GetInstanceMethods (JniPeerMembers.JniInstanceMethods methods) + static ConcurrentDictionary GetInstanceMethods (JniPeerMembers.JniInstanceMethods methods) { var f = typeof (JniPeerMembers.JniInstanceMethods).GetField ("InstanceMethods", BindingFlags.NonPublic | BindingFlags.Instance); - return (Dictionary) f.GetValue (methods); + return (ConcurrentDictionary) f.GetValue (methods); } [Test] diff --git a/tests/Android.Benchmarks/Android.Benchmarks.csproj b/tests/Android.Benchmarks/Android.Benchmarks.csproj new file mode 100644 index 00000000000..514ce0b7051 --- /dev/null +++ b/tests/Android.Benchmarks/Android.Benchmarks.csproj @@ -0,0 +1,22 @@ + + + + $(DotNetAndroidTargetFramework) + $(AndroidMinimumDotNetApiLevel) + Exe + enable + enable + true + net.dot.android.benchmarks + 1 + 1.0 + Xamarin.Android.Benchmarks + apk + false + + + + + + + diff --git a/tests/Android.Benchmarks/AndroidManifest.xml b/tests/Android.Benchmarks/AndroidManifest.xml new file mode 100644 index 00000000000..4f6311983a5 --- /dev/null +++ b/tests/Android.Benchmarks/AndroidManifest.xml @@ -0,0 +1,4 @@ + + + + diff --git a/tests/Android.Benchmarks/BenchmarkInstrumentation.cs b/tests/Android.Benchmarks/BenchmarkInstrumentation.cs new file mode 100644 index 00000000000..2247219cb7c --- /dev/null +++ b/tests/Android.Benchmarks/BenchmarkInstrumentation.cs @@ -0,0 +1,83 @@ +using Android.Runtime; +using BenchmarkDotNet.Columns; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Exporters.Csv; +using BenchmarkDotNet.Filters; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Loggers; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains.InProcess.NoEmit; + +namespace Xamarin.Android.Benchmarks; + +[Instrumentation (Name = "net.dot.android.benchmarks.BenchmarkInstrumentation")] +public class BenchmarkInstrumentation : Instrumentation +{ + protected BenchmarkInstrumentation (IntPtr handle, JniHandleOwnership ownership) + : base (handle, ownership) + { + } + + public override void OnCreate (Bundle? arguments) + { + base.OnCreate (arguments); + Filter = GetFilter (arguments); + Start (); + } + + public override void OnStart () + { + base.OnStart (); + + var results = new Bundle (); + try { + var externalFiles = Application.Context.GetExternalFilesDir (null)?.AbsolutePath; + var artifactsPath = Path.Combine (externalFiles ?? Path.GetTempPath (), "BenchmarkDotNet.Artifacts"); + var config = ManualConfig.CreateEmpty () + .AddJob (Job.ShortRun + .WithToolchain (InProcessNoEmitToolchain.Instance) + .WithId ("Android")) + .AddLogger (ConsoleLogger.Default) + .AddColumnProvider (DefaultColumnProviders.Instance) + .AddExporter (CsvExporter.Default, MarkdownExporter.GitHub) + .WithArtifactsPath (artifactsPath) + .WithOptions (ConfigOptions.DisableOptimizationsValidator); + if (Filter != null) + config.AddFilter (new GlobFilter ([Filter])); + + var summaries = BenchmarkRunner.Run (GetType ().Assembly, config); + var reportCount = summaries.Sum (summary => summary.Reports.Length); + var hasErrors = summaries.Any (summary => summary.HasCriticalValidationErrors); + + results.PutInt ("reports", reportCount); + results.PutString ("artifactsPath", artifactsPath); + Console.WriteLine ($"BENCHMARKS_COMPLETE reports={reportCount} artifacts={artifactsPath}"); + Finish (hasErrors ? Result.Canceled : Result.Ok, results); + } catch (Exception ex) { + results.PutString ("error", ex.ToString ()); + Console.WriteLine ($"BENCHMARKS_FAILED {ex}"); + Finish (Result.Canceled, results); + } + } + + string? Filter { get; set; } + + static string? GetFilter (Bundle? arguments) + { + var filter = arguments?.GetString ("filter"); + if (!string.IsNullOrWhiteSpace (filter)) + return filter; + + var value = arguments?.GetString ("args"); + if (string.IsNullOrWhiteSpace (value)) + return null; + + var values = value.Split (' ', StringSplitOptions.RemoveEmptyEntries); + for (int i = 0; i < values.Length - 1; i++) { + if (values [i] == "--filter") + return values [i + 1]; + } + return null; + } +} diff --git a/tests/Android.Benchmarks/JniMethodInfoBenchmarks.cs b/tests/Android.Benchmarks/JniMethodInfoBenchmarks.cs new file mode 100644 index 00000000000..1b34093d821 --- /dev/null +++ b/tests/Android.Benchmarks/JniMethodInfoBenchmarks.cs @@ -0,0 +1,42 @@ +using BenchmarkDotNet.Attributes; + +namespace Xamarin.Android.Benchmarks; + +public unsafe class JniMethodInfoBenchmarks +{ + const string EncodedMember = "hashCode.()I"; + + readonly Java.Lang.String peer; + readonly Java.Interop.JniPeerMembers.JniInstanceMethods methods; + + public JniMethodInfoBenchmarks () + { + peer = new Java.Lang.String ("benchmark"); + methods = peer.JniPeerMembers.InstanceMethods; + } + + [GlobalSetup] + public void Setup () + { + _ = InvokeWithStringCache (); + _ = InvokeGeneratedBinding (); + } + + [GlobalCleanup] + public void Cleanup () + { + peer.Dispose (); + } + + [Benchmark (Baseline = true)] + public int InvokeWithStringCache () + { + return methods.InvokeVirtualInt32Method (EncodedMember, peer, null); + } + + [Benchmark] + public int InvokeGeneratedBinding () + { + return peer.GetHashCode (); + } +} diff --git a/tests/Android.Benchmarks/README.md b/tests/Android.Benchmarks/README.md new file mode 100644 index 00000000000..f540d1cde96 --- /dev/null +++ b/tests/Android.Benchmarks/README.md @@ -0,0 +1,19 @@ +# Android benchmarks + +This instrumentation-only app runs BenchmarkDotNet in-process on an Android +device. It has no activity. + +Run every benchmark: + +```sh +./dotnet-local.sh run --project tests/Android.Benchmarks/Android.Benchmarks.csproj -c Release +``` + +Filter benchmarks by passing BenchmarkDotNet's `--filter` argument: + +```sh +./dotnet-local.sh run --project tests/Android.Benchmarks/Android.Benchmarks.csproj -c Release -- --filter '*JniMethodInfoBenchmarks*' +``` + +The instrumentation result reports the on-device artifacts directory. Results +are also streamed through logcat by `dotnet run`.