Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] implement a MemoryStreamPool #4251

Merged
merged 2 commits into from
Feb 19, 2020

Commits on Feb 13, 2020

  1. [Xamarin.Android.Build.Tasks] implement a MemoryStreamPool

    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 `_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 could 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
    
    If I drill down to some of the specific targets that would be affected
    here:
    
        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 less `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`
    `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. This reduced it to one parameter.
    jonathanpeppers committed Feb 13, 2020
    Configuration menu
    Copy the full SHA
    472f6c0 View commit details
    Browse the repository at this point in the history

Commits on Feb 14, 2020

  1. Configuration menu
    Copy the full SHA
    21adf51 View commit details
    Browse the repository at this point in the history