[Java.Interop] use Dictionary<string, *>(StringComparer.Ordinal) (#550)

jonathanpeppers · jonpryor · commit f26bc271e054 · 2020-01-06T15:56:51.000-05:00
Context: https://docs.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings#ordinal-string-operations When profiling startup for a Xamarin.Forms app, I noticed some heavy calls to `Dictionary`: Total(ms) Self(ms) Calls Method name 392 49 14826 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:FindEntry (TKey_REF) 377 18 12744 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:TryGetValue (TKey_REF,TValue_REF&) 129 14 3827 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:TryInsert (TKey_REF,TValue_REF,System.Collections.Generic.InsertionBehavior) Quite a few of these are coming from Java.Interop itself, and I noticed we were using the default constructor for ``Dictionary`2``. This means we are using a culture-aware string comparison for every Java member we lookup. I could see a small difference in a Release build of the Xamarin.Forms integration project in xamarin-android: * Before: 01-03 09:26:24.830 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +763ms 01-03 09:26:28.865 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms 01-03 09:26:32.865 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms 01-03 09:26:36.898 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +759ms 01-03 09:26:40.915 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms 01-03 09:26:44.863 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +752ms 01-03 09:26:48.864 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +759ms 01-03 09:26:52.881 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +753ms 01-03 09:26:56.864 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +746ms 01-03 09:27:00.864 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms * Average(ms): 756 * Std Err(ms): 1.60554594383897 * Std Dev(ms): 5.07718207057594 * After: 01-03 09:48:00.954 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +745ms 01-03 09:48:04.988 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms 01-03 09:48:08.987 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +761ms 01-03 09:48:12.953 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +735ms 01-03 09:48:16.987 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +752ms 01-03 09:48:20.971 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +757ms 01-03 09:48:24.938 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms 01-03 09:48:28.921 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +767ms 01-03 09:48:32.888 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +745ms 01-03 09:48:36.922 1473 1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +757ms * Average(ms): 753.3 * Std Err(ms): 2.97040962382856 * Std Dev(ms): 9.39325999498222 I would guess this saves ~2ms during startup? I also toyed with changing the `capacity` constructor value to 0, but that seemed to make things worse. I don't think there is a way for us to "guess" the default capacity for these dictionaries, so let's let the BCL do that.
diff --git a/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs b/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceFields.cs
@@ -13,7 +13,7 @@ internal JniInstanceFields (JniPeerMembers members)
 
 		readonly JniPeerMembers                             Members;
 
-		Dictionary<string, JniFieldInfo>                    InstanceFields  = new Dictionary<string, JniFieldInfo>();
+		Dictionary<string, JniFieldInfo>                    InstanceFields  = new Dictionary<string, JniFieldInfo>(StringComparer.Ordinal);
 
 		internal void Dispose ()
 		{
diff --git a/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs
@@ -35,7 +35,7 @@ internal    JniType                                 JniPeerType {
 
 		readonly Type                                       DeclaringType;
 
-		Dictionary<string, JniMethodInfo>                   InstanceMethods = new Dictionary<string, JniMethodInfo>();
+		Dictionary<string, JniMethodInfo>                   InstanceMethods = new Dictionary<string, JniMethodInfo>(StringComparer.Ordinal);
 		Dictionary<Type, JniInstanceMethods>                SubclassConstructors = new Dictionary<Type, JniInstanceMethods> ();
 
 		internal void Dispose ()
diff --git a/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs b/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticFields.cs
@@ -13,7 +13,7 @@ internal JniStaticFields (JniPeerMembers members)
 
 		readonly JniPeerMembers                             Members;
 
-		Dictionary<string, JniFieldInfo>                    StaticFields  = new Dictionary<string, JniFieldInfo>();
+		Dictionary<string, JniFieldInfo>                    StaticFields  = new Dictionary<string, JniFieldInfo>(StringComparer.Ordinal);
 
 		public JniFieldInfo GetFieldInfo (string encodedMember)
 		{
diff --git a/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs
@@ -13,7 +13,7 @@ internal JniStaticMethods (JniPeerMembers members)
 
 		internal    readonly    JniPeerMembers              Members;
 
-		Dictionary<string, JniMethodInfo>                   StaticMethods   = new Dictionary<string, JniMethodInfo>();
+		Dictionary<string, JniMethodInfo>                   StaticMethods   = new Dictionary<string, JniMethodInfo>(StringComparer.Ordinal);
 
 		internal void Dispose ()
 		{

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ internal JniInstanceFields (JniPeerMembers members)`
`13`	`13`
`14`	`14`	`readonly JniPeerMembers Members;`
`15`	`15`
`16`		`- Dictionary<string, JniFieldInfo> InstanceFields = new Dictionary<string, JniFieldInfo>();`
	`16`	`+ Dictionary<string, JniFieldInfo> InstanceFields = new Dictionary<string, JniFieldInfo>(StringComparer.Ordinal);`
`17`	`17`
`18`	`18`	`internal void Dispose ()`
`19`	`19`	`{`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ internal JniStaticFields (JniPeerMembers members)`
`13`	`13`
`14`	`14`	`readonly JniPeerMembers Members;`
`15`	`15`
`16`		`- Dictionary<string, JniFieldInfo> StaticFields = new Dictionary<string, JniFieldInfo>();`
	`16`	`+ Dictionary<string, JniFieldInfo> StaticFields = new Dictionary<string, JniFieldInfo>(StringComparer.Ordinal);`
`17`	`17`
`18`	`18`	`public JniFieldInfo GetFieldInfo (string encodedMember)`
`19`	`19`	`{`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ internal JniStaticMethods (JniPeerMembers members)`
`13`	`13`
`14`	`14`	`internal readonly JniPeerMembers Members;`
`15`	`15`
`16`		`- Dictionary<string, JniMethodInfo> StaticMethods = new Dictionary<string, JniMethodInfo>();`
	`16`	`+ Dictionary<string, JniMethodInfo> StaticMethods = new Dictionary<string, JniMethodInfo>(StringComparer.Ordinal);`
`17`	`17`
`18`	`18`	`internal void Dispose ()`
`19`	`19`	`{`