Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] implement a MemoryStreamPool (#4251)
Browse files Browse the repository at this point in the history
Context: https://docs.microsoft.com/dotnet/api/system.buffers.arraypool-1

When using `MemoryStream`, it is better to reuse a `MemoryStream`
instance rather than creating new ones, such as in a loop:

	using (var memoryStream = new MemoryStream ())
	    foreach (var foo in bar) {
	        //Reset for reuse
	        memoryStream.SetLength (0);
	        // Use the stream
	    }
	}

The `SetLength(0)` call preserves the underlying `byte[]` and just
sets the `_length` and `_position` fields.  Subsequent writes don't
need to allocate another `byte[]`:

	https://github.com/mono/mono/blob/eb85a108a33ba86ffd184689b62ac1f7250c9818/mcs/class/referencesource/mscorlib/system/io/memorystream.cs#L527-L548

To make this pattern even better, we can model after
`System.Buffers.ArrayPool` and reuse `MemoryStream` objects throughout
the entire build.

	var memoryStream = MemoryStreamPool.Shared.Rent ();
	try {
	    // Use the stream
	} finally {
	    MemoryStreamPool.Shared.Return (memoryStream);
	}

Note that you will have to take special care to not dispose the
`MemoryStream`.  If we `Return()` a disposed `MemoryStream`, that
would be bad news!

In many cases a `StreamWriter` or `TextWriter` are wrapped around a
`MemoryStream`, so we implement a convenience method:

	using (var writer = MemoryStreamPool.Shared.CreateStreamWriter ()) {
	    // Use the writer
	    writer.Flush ();
	    MonoAndroidHelper.CopyIfStreamChanged (writer.BaseStream, path);
	}

In this case `writer` is a special `StreamWriter` that returns the
underlying `MemoryStream` when the writer is disposed.  It also takes
care of *not* disposing the `MemoryStream` for you.

~~ Results ~~

The reuse of `MemoryStream` impacts the entire build.  Testing on
macOS, a build of the Xamarin.Forms integration project:

	./bin/Release/bin/xabuild tests/Xamarin.Forms-Performance-Integration/Droid/Xamarin.Forms.Performance.Integration.Droid.csproj /restore

Some of the specific targets that would be affected:

  * Before:

        1618 ms  _GenerateJavaStubs                         1 calls
        1656 ms  _ResolveLibraryProjectImports              1 calls
          40 ms  _GeneratePackageManagerJava                1 calls

  * After:

        1568 ms  _GenerateJavaStubs                         1 calls
        1376 ms  _ResolveLibraryProjectImports              1 calls
          36 ms  _GeneratePackageManagerJava                1 calls

This seems like we could save ~334ms on incremental builds where
these targets run.  An example would be an incremental build where
`MainActivity.cs` changed.

If I compare the memory usage with the Mono profiler:

  * Before:

        Allocation summary
            Bytes      Count  Average Type name
        166246184      37846     4392 System.Byte[]
            82480       1031       80 System.IO.MemoryStream

  * After:

        Allocation summary
            Bytes      Count  Average Type name
        157191784      37794     4159 System.Byte[]
            77520        969       80 System.IO.MemoryStream

It seems like we created ~62 fewer `MemoryStream` in this build and
saved ~9,059,360 bytes of allocations.

After a few runs, I found I could drop the numbers on some of the
times in `MSBuildDeviceIntegration.csv`.  The times seemed to improve
a bit for incremental builds where a lot is happening, such as when
the `.csproj` changes.  Some of the time improved is likely other
changes around `<ResolveAssemblies/>` or `<ResolveAssemblyReference/>`
such as 97d250b or 1e96c79.

~~ General Refactoring ~~

Any usage of `new Utf8Encoding(false)` I moved to a `static`
`MonoAndroidHelper.UTF8withoutBOM` field.

The method `Generator.CreateJavaSources()` had eight parameters!
I moved it to be an instance method on `<GenerateJavaStubs/>`, which
could use the properties of the task, reducing it to one parameter.
  • Loading branch information
jonathanpeppers authored Feb 19, 2020
1 parent 771ee81 commit 87ee20c
Show file tree
Hide file tree
Showing 21 changed files with 385 additions and 228 deletions.
100 changes: 0 additions & 100 deletions src/Xamarin.Android.Build.Tasks/Generator/Generator.cs

This file was deleted.

2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ bool RunAotCompiler (string assembliesPath, string aotCompiler, string aotOption
var stdout_completed = new ManualResetEvent (false);
var stderr_completed = new ManualResetEvent (false);

using (var sw = new StreamWriter (responseFile, append: false, encoding: new UTF8Encoding (encoderShouldEmitUTF8Identifier: false))) {
using (var sw = new StreamWriter (responseFile, append: false, encoding: MonoAndroidHelper.UTF8withoutBOM)) {
sw.WriteLine (aotOptions + " " + QuoteFileName (assembly));
}

Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/ClassParse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class ClassParse : AndroidTask
public override bool RunTask ()
{
using (var output = new StreamWriter (OutputFile, append: false,
encoding: new UTF8Encoding (encoderShouldEmitUTF8Identifier: false))) {
encoding: MonoAndroidHelper.UTF8withoutBOM)) {
Bytecode.Log.OnLog = LogEventHandler;
var classPath = new Bytecode.ClassPath () {
ApiSource = "class-parse",
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/CompileToDalvik.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ protected override string GenerateCommandLineCommands ()
cmd.AppendSwitchIfNotNull ("--output ", Path.GetDirectoryName (ClassesOutputDirectory));

using (var sw = new StreamWriter (path: inputListFile, append: false,
encoding: new UTF8Encoding (encoderShouldEmitUTF8Identifier: false))) {
encoding: MonoAndroidHelper.UTF8withoutBOM)) {
// .jar files
if (AlternativeJarFiles != null && AlternativeJarFiles.Any ()) {
Log.LogDebugMessage (" processing AlternativeJarFiles...");
Expand Down
112 changes: 91 additions & 21 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Xml.Linq;
using System.Reflection;
using System.Text;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
Expand Down Expand Up @@ -171,7 +171,7 @@ void Run (DirectoryAssemblyResolver res)

// Step 2 - Generate type maps
// Type mappings need to use all the assemblies, always.
WriteTypeMappings (res, allJavaTypes);
WriteTypeMappings (allJavaTypes);

var javaTypes = new List<TypeDefinition> ();
foreach (TypeDefinition td in allJavaTypes) {
Expand All @@ -183,15 +183,7 @@ void Run (DirectoryAssemblyResolver res)
}

// Step 3 - Generate Java stub code
var success = Generator.CreateJavaSources (
Log,
javaTypes,
Path.Combine (OutputDirectory, "src"),
ApplicationJavaClass,
AndroidSdkPlatform,
UseSharedRuntime,
int.Parse (AndroidSdkPlatform) <= 10,
hasExportReference);
var success = CreateJavaSources (javaTypes);
if (!success)
return;

Expand All @@ -202,9 +194,7 @@ void Run (DirectoryAssemblyResolver res)
var managedConflicts = new Dictionary<string, List<string>> (0, StringComparer.Ordinal);
var javaConflicts = new Dictionary<string, List<string>> (0, StringComparer.Ordinal);

// Allocate a MemoryStream with a reasonable guess at its capacity
using (var stream = new MemoryStream (javaTypes.Count * 32))
using (var acw_map = new StreamWriter (stream)) {
using (var acw_map = MemoryStreamPool.Shared.CreateStreamWriter (Encoding.Default)) {
foreach (TypeDefinition type in javaTypes) {
string managedKey = type.FullName.Replace ('/', '.');
string javaKey = JavaNativeTypeManager.ToJniName (type).Replace ('/', '.');
Expand Down Expand Up @@ -246,7 +236,7 @@ void Run (DirectoryAssemblyResolver res)
}

acw_map.Flush ();
MonoAndroidHelper.CopyIfStreamChanged (stream, AcwMapFile);
MonoAndroidHelper.CopyIfStreamChanged (acw_map.BaseStream, AcwMapFile);
}

foreach (var kvp in managedConflicts) {
Expand Down Expand Up @@ -277,11 +267,9 @@ void Run (DirectoryAssemblyResolver res)

var additionalProviders = manifest.Merge (Log, allJavaTypes, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments);

using (var stream = new MemoryStream ()) {
manifest.Save (Log, stream);

// Only write the new manifest if it actually changed
MonoAndroidHelper.CopyIfStreamChanged (stream, MergedAndroidManifestOutput);
// Only write the new manifest if it actually changed
if (manifest.SaveIfChanged (Log, MergedAndroidManifestOutput)) {
Log.LogDebugMessage ($"Saving: {MergedAndroidManifestOutput}");
}

// Create additional runtime provider java sources.
Expand Down Expand Up @@ -312,6 +300,88 @@ void Run (DirectoryAssemblyResolver res)
template => template.Replace ("// REGISTER_APPLICATION_AND_INSTRUMENTATION_CLASSES_HERE", regCallsWriter.ToString ()));
}

bool CreateJavaSources (IEnumerable<TypeDefinition> javaTypes)
{
string outputPath = Path.Combine (OutputDirectory, "src");
string monoInit = GetMonoInitSource (AndroidSdkPlatform, UseSharedRuntime);
bool hasExportReference = ResolvedAssemblies.Any (assembly => Path.GetFileName (assembly.ItemSpec) == "Mono.Android.Export.dll");
bool generateOnCreateOverrides = int.Parse (AndroidSdkPlatform) <= 10;

bool ok = true;
foreach (var t in javaTypes) {
using (var writer = MemoryStreamPool.Shared.CreateStreamWriter ()) {
try {
var jti = new JavaCallableWrapperGenerator (t, Log.LogWarning) {
GenerateOnCreateOverrides = generateOnCreateOverrides,
ApplicationJavaClass = ApplicationJavaClass,
MonoRuntimeInitialization = monoInit,
};

jti.Generate (writer);
writer.Flush ();

var path = jti.GetDestinationPath (outputPath);
MonoAndroidHelper.CopyIfStreamChanged (writer.BaseStream, path);
if (jti.HasExport && !hasExportReference)
Diagnostic.Error (4210, Properties.Resources.XA4210);
} catch (XamarinAndroidException xae) {
ok = false;
Log.LogError (
subcategory: "",
errorCode: "XA" + xae.Code,
helpKeyword: string.Empty,
file: xae.SourceFile,
lineNumber: xae.SourceLine,
columnNumber: 0,
endLineNumber: 0,
endColumnNumber: 0,
message: xae.MessageWithoutCode,
messageArgs: new object [0]
);
} catch (DirectoryNotFoundException ex) {
ok = false;
if (OS.IsWindows) {
Diagnostic.Error (5301, Properties.Resources.XA5301, t.FullName, ex);
} else {
Diagnostic.Error (4209, Properties.Resources.XA4209, t.FullName, ex);
}
} catch (Exception ex) {
ok = false;
Diagnostic.Error (4209, Properties.Resources.XA4209, t.FullName, ex);
}
}
}
return ok;
}

static string GetMonoInitSource (string androidSdkPlatform, bool useSharedRuntime)
{
// Lookup the mono init section from MonoRuntimeProvider:
// Mono Runtime Initialization {{{
// }}}
var builder = new StringBuilder ();
var runtime = useSharedRuntime ? "Shared" : "Bundled";
var api = "";
if (int.TryParse (androidSdkPlatform, out int apiLevel) && apiLevel < 21) {
api = ".20";
}
var assembly = Assembly.GetExecutingAssembly ();
using (var s = assembly.GetManifestResourceStream ($"MonoRuntimeProvider.{runtime}{api}.java"))
using (var reader = new StreamReader (s)) {
bool copy = false;
string line;
while ((line = reader.ReadLine ()) != null) {
if (string.CompareOrdinal ("\t\t// Mono Runtime Initialization {{{", line) == 0)
copy = true;
if (copy)
builder.AppendLine (line);
if (string.CompareOrdinal ("\t\t// }}}", line) == 0)
break;
}
}
return builder.ToString ();
}

string GetResource (string resource)
{
using (var stream = GetType ().Assembly.GetManifestResourceStream (resource))
Expand All @@ -326,7 +396,7 @@ void SaveResource (string resource, string filename, string destDir, Func<string
MonoAndroidHelper.CopyIfStringChanged (template, Path.Combine (destDir, filename));
}

void WriteTypeMappings (DirectoryAssemblyResolver resolver, List<TypeDefinition> types)
void WriteTypeMappings (List<TypeDefinition> types)
{
var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis);
if (!tmg.Generate (types, TypemapOutputDirectory, GenerateNativeAssembly))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ void GenerateJava (Package package)
}

var lines = LoadValues (package);
using (var memory = new MemoryStream ())
using (var writer = new StreamWriter (memory, Encoding)) {
using (var writer = MemoryStreamPool.Shared.CreateStreamWriter ()) {
// This code is based on the Android gradle plugin
// https://android.googlesource.com/platform/tools/base/+/908b391a9c006af569dfaff08b37f8fdd6c4da89/build-system/builder/src/main/java/com/android/builder/internal/SymbolWriter.java

Expand Down Expand Up @@ -173,7 +172,7 @@ void GenerateJava (Package package)

writer.Flush ();
var r_java = Path.Combine (output_directory, package.Name.Replace ('.', Path.DirectorySeparatorChar), "R.java");
if (MonoAndroidHelper.CopyIfStreamChanged (memory, r_java)) {
if (MonoAndroidHelper.CopyIfStreamChanged (writer.BaseStream, r_java)) {
LogDebugMessage ($"Writing: {r_java}");
} else {
LogDebugMessage ($"Up to date: {r_java}");
Expand Down Expand Up @@ -227,8 +226,6 @@ bool SetValue (string key, string[] line, string r_txt)
}
return false;
}

static readonly Encoding Encoding = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false);
static readonly char [] Delimiter = new [] { ' ' };

class Index
Expand Down
Loading

0 comments on commit 87ee20c

Please sign in to comment.