Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Xamarin.Android.Build.Tasks] implement a MemoryStreamPool (#4251)
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