[Java.Interop] Avoid locks for cached JNI member lookups

Xamarin.Android-Tests.slnx

@@ -27,6 +27,7 @@
     <Project Path="samples/HelloWorld/HelloWorld/HelloWorld.DotNet.csproj" />
   </Folder>
   <Folder Name="/tests/">
+    <Project Path="tests/Android.Benchmarks/Android.Benchmarks.csproj" />
     <Project Path="tests/StartupHook/StartupHook.csproj" />
   </Folder>
   <Folder Name="/tests/Mono.Android-Tests/" />

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<string, JniFieldInfo>                    InstanceFields  = new Dictionary<string, JniFieldInfo>(StringComparer.Ordinal);
+		readonly ConcurrentDictionary<string, JniFieldInfo> InstanceFields = new ConcurrentDictionary<string, JniFieldInfo> (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);
 		}
 	}}
 }
-

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<string, JniMethodInfo>                   InstanceMethods = new Dictionary<string, JniMethodInfo>(StringComparer.Ordinal);
-		Dictionary<Type, JniInstanceMethods>                SubclassConstructors = new Dictionary<Type, JniInstanceMethods> ();
+		readonly ConcurrentDictionary<string, JniMethodInfo>    InstanceMethods      = new ConcurrentDictionary<string, JniMethodInfo> (1, 3, StringComparer.Ordinal);
+		readonly ConcurrentDictionary<Type, JniInstanceMethods> SubclassConstructors = new ConcurrentDictionary<Type, JniInstanceMethods> (1, 1);
 
 		internal void Dispose ()
 		{
@@ -58,27 +58,16 @@ 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)
 		{
 			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)

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<string, JniFieldInfo>                    StaticFields  = new Dictionary<string, JniFieldInfo>(StringComparer.Ordinal);
+		readonly ConcurrentDictionary<string, JniFieldInfo> StaticFields = new ConcurrentDictionary<string, JniFieldInfo> (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 ()
 		}
 	}}
 }
-

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<string, JniMethodInfo>                   StaticMethods   = new Dictionary<string, JniMethodInfo>(StringComparer.Ordinal);
+		readonly ConcurrentDictionary<string, JniMethodInfo> StaticMethods = new ConcurrentDictionary<string, JniMethodInfo> (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
 		}
 	}}
 }
-

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<string, JniMethodInfo> GetInstanceMethods (JniPeerMembers.JniInstanceMethods methods)
+		static ConcurrentDictionary<string, JniMethodInfo> GetInstanceMethods (JniPeerMembers.JniInstanceMethods methods)
 		{
 			var f   = typeof (JniPeerMembers.JniInstanceMethods).GetField ("InstanceMethods", BindingFlags.NonPublic | BindingFlags.Instance);
-			return (Dictionary<string, JniMethodInfo>) f.GetValue (methods);
+			return (ConcurrentDictionary<string, JniMethodInfo>) f.GetValue (methods);
 		}
 
 		[Test]

tests/Android.Benchmarks/Android.Benchmarks.csproj

@@ -0,0 +1,22 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>$(DotNetAndroidTargetFramework)</TargetFramework>
+    <SupportedOSPlatformVersion>$(AndroidMinimumDotNetApiLevel)</SupportedOSPlatformVersion>
+    <OutputType>Exe</OutputType>
+    <Nullable>enable</Nullable>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <ApplicationId>net.dot.android.benchmarks</ApplicationId>
+    <ApplicationVersion>1</ApplicationVersion>
+    <ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
+    <RootNamespace>Xamarin.Android.Benchmarks</RootNamespace>
+    <AndroidPackageFormat>apk</AndroidPackageFormat>
+    <IsPackable>false</IsPackable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="BenchmarkDotNet" Version="0.15.8" />
+  </ItemGroup>
+
+</Project>

tests/Android.Benchmarks/AndroidManifest.xml

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android">
+  <application android:label=".NET for Android Benchmarks" />
+</manifest>

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;
+	}
+}