Improve sort function in crossgen2 (#36676)
authorDavid Wrighton <davidwr@microsoft.com>
Tue, 19 May 2020 18:12:20 +0000 (11:12 -0700)
committerGitHub <noreply@github.com>
Tue, 19 May 2020 18:12:20 +0000 (11:12 -0700)
- Increase memory locality substantially in composite images
- Sort non-generic methods with module together
- Sort generic methods near other generic methods with similar instantiations

Json benchmark showed improvement from 237,827 RPS to 270,306 RPS, and reduced working set by about 10MB

src/coreclr/src/tools/Common/TypeSystem/Ecma/EcmaMethod.Sorting.cs
src/coreclr/src/tools/Common/TypeSystem/Ecma/EcmaType.Sorting.cs
src/coreclr/src/tools/Common/TypeSystem/Sorting/InstantiatedMethod.Sorting.cs
src/coreclr/src/tools/Common/TypeSystem/Sorting/InstantiatedType.Sorting.cs

index b9cfbda..c5e20b5 100644 (file)
@@ -17,12 +17,14 @@ namespace Internal.TypeSystem.Ecma
 
             EcmaModule module = _type.EcmaModule;
             EcmaModule otherModule = otherMethod._type.EcmaModule;
-            
-            int result = module.MetadataReader.GetToken(_handle) - otherModule.MetadataReader.GetToken(otherMethod._handle);
+
+            // Sort by module in preference to by token. This will place methods of the same type near each other
+            // even when working with several modules
+            int result = module.CompareTo(otherModule);
             if (result != 0)
                 return result;
 
-            return module.CompareTo(otherModule);
+            return module.MetadataReader.GetToken(_handle) - otherModule.MetadataReader.GetToken(otherMethod._handle);
         }
     }
 }
index 246f36a..77a3fbe 100644 (file)
@@ -13,12 +13,14 @@ namespace Internal.TypeSystem.Ecma
 
         protected internal override int CompareToImpl(TypeDesc other, TypeSystemComparer comparer)
         {
+            // Sort by module in preference to by token. This will place types from the same module near each other
+            // even when working with several modules.
             var otherType = (EcmaType)other;
-            int result = _module.MetadataReader.GetToken(_handle) - otherType._module.MetadataReader.GetToken(otherType._handle);
+            int result = _module.CompareTo(otherType._module);
             if (result != 0)
                 return result;
 
-            return _module.CompareTo(otherType._module);
+            return _module.MetadataReader.GetToken(_handle) - otherType._module.MetadataReader.GetToken(otherType._handle);
         }
     }
 }
index 17b2707..065c122 100644 (file)
@@ -12,21 +12,27 @@ namespace Internal.TypeSystem
         protected internal override int CompareToImpl(MethodDesc other, TypeSystemComparer comparer)
         {
             var otherMethod = (InstantiatedMethod)other;
-            int result = _instantiation.Length - otherMethod._instantiation.Length;
-            if (result != 0)
-                return result;
-
-            result = comparer.Compare(_methodDef, otherMethod._methodDef);
-            if (result != 0)
-                return result;
-
+            // Sort by instantiation before sorting by associated method definition
+            // The goal of this is to keep methods which work with the same types near
+            // to each other. This is a better heuristic than sorting by method definition
+            // then by instantiation.
+            //
+            // The goal is to sort methods like SomeClass.SomeMethod<UserStruct>, 
+            // near SomeOtherClass.SomeOtherMethod<UserStruct, int>
+            int result = 0;
+            // Sort instantiations of the same type together
             for (int i = 0; i < _instantiation.Length; i++)
             {
+                if (i >= otherMethod._instantiation.Length)
+                    return 1;
                 result = comparer.Compare(_instantiation[i], otherMethod._instantiation[i]);
                 if (result != 0)
-                    break;
+                    return result;
             }
+            if (_instantiation.Length < otherMethod._instantiation.Length)
+                return -1;
 
+            result = comparer.Compare(_methodDef, otherMethod._methodDef);
             return result;
         }
     }
index f749b45..b983382 100644 (file)
@@ -14,20 +14,28 @@ namespace Internal.TypeSystem
         protected internal override int CompareToImpl(TypeDesc other, TypeSystemComparer comparer)
         {
             var otherType = (InstantiatedType)other;
+            // Sort by instantiation before sorting by associated method definition
+            // The goal of this is to keep methods which work with the same types near
+            // to each other. This is a better heuristic than sorting by method definition
+            // then by instantiation.
+            //
+            // The goal is to sort classes like SomeClass<UserStruct>, 
+            // near SomeOtherClass<UserStruct, int>
 
-            int result = comparer.Compare(_typeDef, otherType._typeDef);
-            if (result == 0)
+            int result = 0;
+            // Sort instantiations of the same type together
+            for (int i = 0; i < _instantiation.Length; i++)
             {
-                Debug.Assert(_instantiation.Length == otherType._instantiation.Length);
-                for (int i = 0; i < _instantiation.Length; i++)
-                {
-                    result = comparer.Compare(_instantiation[i], otherType._instantiation[i]);
-                    if (result != 0)
-                        break;
-                }
+                if (i >= otherType._instantiation.Length)
+                    return 1;
+                result = comparer.Compare(_instantiation[i], otherType._instantiation[i]);
+                if (result != 0)
+                    return result;
             }
+            if (_instantiation.Length < otherType._instantiation.Length)
+                return -1;
 
-            return result;
+            return comparer.Compare(_typeDef, otherType._typeDef);
         }
     }
 }