Skip to content

Commit 43e1cdc

Browse files
[linker] shortcircuit to prevent string.Format calls
I was using the Mono profiler for our build, and I noticed: Allocation summary Bytes Count Average Type name 180765632 2142803 84 System.String 53218472 bytes from: MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.AssemblyDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethodsUnconditional (Mono.Cecil.AssemblyDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.TypeDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameSignature (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition) (wrapper managed-to-native) string:FastAllocateString (int) ~53MB of string from this one method is a lot! I found that one of these `string.Format` calls are running for almost every method on every interface in every assembly: (string.Format ("{0}.{1}", iface.FullName, iMethod.Name) != tMethod.Name) : (string.Format ("{0}.{1}.{2}", iMethod.DeclaringType.DeclaringType, iface.Name, iMethod.Name) != tMethod.Name)) Even the simplest non-matching cases would call `string.Format`: iMethod.Name == "Foo" tMethod.Name == "Bar" Since this if-statement is quite long, I refactored it into a new `HaveSameMethodName` method to match the `HaveSameSignature` method. I added a simple check that would skip the `string.Format` calls: if (tMethod.Name.IndexOf (".", StringComparison.Ordinal) == -1) return false; If there is no ".", we don't need to append strings that would contain one. There is no way the `==` would match in that case. ~~ Results ~~ An initial build of the Xamarin.Forms integration project on Windows/.NET framework: Before: 796 ms LinkAssembliesNoShrink 1 calls After: 767 ms LinkAssembliesNoShrink 1 calls The same project on macOS/Mono (slightly slower machine, too): Before: 1341 ms LinkAssembliesNoShrink 1 calls After: 1025 ms LinkAssembliesNoShrink 1 calls String allocations are way better, too: Before: Allocation summary Bytes Count Average Type name 180765632 2142803 84 System.String After: Allocation summary Bytes Count Average Type name 127736832 1652070 77 System.String Shows 53,028,800 bytes saved. Before: 53218472 bytes from: MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.AssemblyDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethodsUnconditional (Mono.Cecil.AssemblyDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.TypeDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameSignature (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition) After: 1933624 bytes from: MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethodsUnconditional (Mono.Cecil.AssemblyDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.TypeDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameSignature (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition) MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameMethodName (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition) Shows 51,284,848 bytes saved. I don't know about the discrepancy between these two sets of numbers. But I think this is roughly ~50MB less string allocations.
1 parent b32d7f3 commit 43e1cdc

File tree

2 files changed

+84
-1
lines changed

2 files changed

+84
-1
lines changed

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ bool HaveSameSignature (TypeReference iface, MethodDefinition iMethod, MethodDef
158158
if (IsInOverrides (iMethod, tMethod))
159159
return true;
160160

161-
if (iMethod.Name != tMethod.Name && (iMethod.DeclaringType == null || (iMethod.DeclaringType.DeclaringType == null ? (string.Format ("{0}.{1}", iface.FullName, iMethod.Name) != tMethod.Name) : (string.Format ("{0}.{1}.{2}", iMethod.DeclaringType.DeclaringType, iface.Name, iMethod.Name) != tMethod.Name))))
161+
if (!HaveSameMethodName (iface, iMethod, tMethod))
162162
return false;
163163

164164
if (!CompareTypes (iMethod.MethodReturnType.ReturnType, tMethod.MethodReturnType.ReturnType))
@@ -189,6 +189,20 @@ bool HaveSameSignature (TypeReference iface, MethodDefinition iMethod, MethodDef
189189
return true;
190190
}
191191

192+
bool HaveSameMethodName (TypeReference iface, MethodDefinition iMethod, MethodDefinition tMethod)
193+
{
194+
if (iMethod.Name == tMethod.Name)
195+
return true;
196+
if (tMethod.Name.IndexOf (".", StringComparison.Ordinal) == -1)
197+
return false;
198+
var declaringType = iMethod.DeclaringType?.DeclaringType;
199+
if (declaringType == null) {
200+
return $"{iface.FullName}.{iMethod.Name}" == tMethod.Name;
201+
} else {
202+
return $"{iMethod.DeclaringType.DeclaringType}.{iface.Name}.{iMethod.Name}" == tMethod.Name;
203+
}
204+
}
205+
192206
bool FixAbstractMethods (TypeDefinition type)
193207
{
194208
if (!type.HasInterfaces)

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,75 @@ static void CreateAbstractIfaceImplementation (string assemblyPath, AssemblyDefi
7070
}
7171
}
7272

73+
[Test]
74+
public void FixAbstractMethodsStep_NestedTypes ()
75+
{
76+
var path = Path.Combine (Path.GetFullPath (XABuildPaths.TestOutputDirectory), "temp", TestName);
77+
var step = new FixAbstractMethodsStep ();
78+
var pipeline = new Pipeline ();
79+
80+
Directory.CreateDirectory (path);
81+
82+
using (var context = new LinkContext (pipeline)) {
83+
84+
context.Resolver.AddSearchDirectory (path);
85+
86+
var myAssemblyPath = Path.Combine (path, "MyAssembly.dll");
87+
88+
using (var android = CreateFauxMonoAndroidAssembly ()) {
89+
android.Write (Path.Combine (path, "Mono.Android.dll"));
90+
CreateNestedInterface (myAssemblyPath, android);
91+
}
92+
93+
using (var assm = context.Resolve (myAssemblyPath)) {
94+
step.Process (context);
95+
96+
var impl = assm.MainModule.GetType ("MyNamespace.Nested/MyClass");
97+
var method = impl.Methods.FirstOrDefault (m => m.Name == "MyImplementedMethod");
98+
Assert.IsNotNull (method, "MyImplementedMethod should exist");
99+
Assert.AreEqual (0, method.Body.Instructions.Count, "MyImplementedMethod should be an empty method");
100+
method = impl.Methods.FirstOrDefault (m => m.Name == "MyMissingMethod");
101+
Assert.IsNotNull (method, "MyMissingMethod should exist");
102+
Assert.AreNotEqual (0, method.Body.Instructions.Count, "MyMissingMethod should *not* be an empty method");
103+
}
104+
}
105+
106+
Directory.Delete (path, true);
107+
}
108+
109+
static void CreateNestedInterface (string assemblyPath, AssemblyDefinition android)
110+
{
111+
using (var assm = AssemblyDefinition.CreateAssembly (new AssemblyNameDefinition ("NestedIFaceTest", new Version ()), "NestedIFaceTest", ModuleKind.Dll)) {
112+
var void_type = assm.MainModule.ImportReference (typeof (void));
113+
var int_type = assm.MainModule.ImportReference (typeof (int));
114+
115+
// Create nested interface
116+
var nested = new TypeDefinition ("MyNamespace", "Nested", TypeAttributes.Class);
117+
var iface = new TypeDefinition (null, "IMyInterface", TypeAttributes.Interface);
118+
iface.DeclaringType = nested;
119+
nested.NestedTypes.Add (iface);
120+
121+
var impl_method = new MethodDefinition ("MyImplementedMethod", MethodAttributes.Abstract, void_type);
122+
iface.Methods.Add (impl_method);
123+
var missing_method = new MethodDefinition ("MyMissingMethod", MethodAttributes.Abstract, void_type);
124+
iface.Methods.Add (missing_method);
125+
assm.MainModule.Types.Add (nested);
126+
assm.MainModule.Types.Add (iface);
127+
128+
// Create nested implementing class
129+
var jlo = assm.MainModule.ImportReference (android.MainModule.GetType ("Java.Lang.Object"));
130+
var impl = new TypeDefinition (null, "MyClass", TypeAttributes.Public, jlo);
131+
impl.Interfaces.Add (new InterfaceImplementation (iface));
132+
impl.DeclaringType = nested;
133+
nested.NestedTypes.Add (impl);
134+
135+
impl.Methods.Add (new MethodDefinition (impl_method.Name, MethodAttributes.Public, void_type));
136+
137+
assm.MainModule.Types.Add (impl);
138+
assm.Write (assemblyPath);
139+
}
140+
}
141+
73142
static AssemblyDefinition CreateFauxMonoAndroidAssembly ()
74143
{
75144
var assm = AssemblyDefinition.CreateAssembly (new AssemblyNameDefinition ("Mono.Android", new Version ()), "DimTest", ModuleKind.Dll);

0 commit comments

Comments
 (0)