Skip to content

Commit

Permalink
Use System.Buffers.ArrayPool
Browse files Browse the repository at this point in the history 
When profiling xamarin-android builds with the Mono profiler, I noticed
`libZipSharp` is one of the biggest allocators of `byte[]`:

    Allocation summary
        Bytes      Count  Average Type name
    251089192      57759     4347 System.Byte[]
        94852584 bytes from:
            Xamarin.Tools.Zip.ZipArchive:Close ()
            (wrapper managed-to-native) Xamarin.Tools.Zip.Native:zip_close (intptr)
            (wrapper native-to-managed) Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
            Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
            (wrapper alloc) object:ProfilerAllocVector (intptr,intptr)
            (wrapper managed-to-native) object:__icall_wrapper_mono_gc_alloc_vector (intptr,intptr,intptr)
        25673176 bytes from:
            Xamarin.Android.Tasks.ResolveLibraryProjectImports:Extract (System.Collections.Generic.IDictionary`2<string, Microsoft.Build.Framework.ITaskItem>,System.Collections.Generic.ICollection`1<Microsoft.Build.Framework.ITaskItem>,System.Collections.Generic.ICollection`1<Microsoft.Build.Framework.ITaskItem>,System.Collections.Generic.ICollection`1<Microsoft.Build.Framework.ITaskItem>)
            Xamarin.Android.Tools.Files:ExtractAll (Xamarin.Tools.Zip.ZipArchive,string,System.Action`2<int, int>,System.Func`2<string, string>,System.Func`2<string, bool>)
            Xamarin.Tools.Zip.ZipEntry:Extract (System.IO.Stream)
            Xamarin.Tools.Zip.ZipEntry:DoExtract (intptr,System.IO.Stream,Xamarin.Tools.Zip.EntryExtractEventArgs)
            (wrapper alloc) object:ProfilerAllocVector (intptr,intptr)
            (wrapper managed-to-native) object:__icall_wrapper_mono_gc_alloc_vector (intptr,intptr,intptr)
        3780312 bytes from:
            Xamarin.Tools.Zip.ZipEntry:DoExtract (intptr,System.IO.Stream,Xamarin.Tools.Zip.EntryExtractEventArgs)
            (wrapper managed-to-native) Xamarin.Tools.Zip.Native:zip_fread (intptr,byte[],ulong)
            (wrapper native-to-managed) Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
            Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
            (wrapper alloc) object:ProfilerAllocVector (intptr,intptr)
            (wrapper managed-to-native) object:__icall_wrapper_mono_gc_alloc_vector (intptr,intptr,intptr)

This seems like *a lot* of `byte[]` allocations.

To improve this, I added a `<PackageReference/>` to `System.Buffers`
and could make use of `ArrayPool`.

`libZipSharp` will likely still allocate some large `byte[]`, but things
should improve because many will be reused.

The changes appear to have saved ~122,523,744 bytes of allocations:

    Allocation summary
        Bytes      Count  Average Type name
    Before:
    251089192      57759     4347 System.Byte[]
    After:
    128565448      31764     4047 System.Byte[]

If I compare `stream_callback`, it has massively less allocations:

    Before:
    94852584 bytes from:
        Xamarin.Tools.Zip.ZipArchive:Close ()
        (wrapper managed-to-native) Xamarin.Tools.Zip.Native:zip_close (intptr)
        (wrapper native-to-managed) Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
        Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
        (wrapper alloc) object:ProfilerAllocVector (intptr,intptr)
        (wrapper managed-to-native) object:__icall_wrapper_mono_gc_alloc_vector (intptr,intptr,intptr)
    After:
    243168 bytes from:
        (wrapper native-to-managed) Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
        Xamarin.Tools.Zip.ZipArchive:stream_callback (intptr,intptr,ulong,Xamarin.Tools.Zip.SourceCommand)
        System.Buffers.DefaultArrayPool`1<byte>:Rent (int)
        System.Buffers.DefaultArrayPool`1/Bucket<byte>:Rent ()
        (wrapper alloc) object:ProfilerAllocVector (intptr,intptr)
        (wrapper managed-to-native) object:__icall_wrapper_mono_gc_alloc_vector (intptr,intptr,intptr)

I saw build performance improvements for the Xamarin.Forms integration
project, the two tasks heavily using `libZipSharp`:

    Before:
    1881 ms  ResolveLibraryProjectImports               1 calls
    3406 ms  BuildApk                                   1 calls
    After:
    1795 ms  ResolveLibraryProjectImports               1 calls
    3150 ms  BuildApk                                   1 calls

I would guess this saves ~350ms on an initial build. Incremental builds
won't be allocating `byte[]` as heavily, but should see some improvement.

The only drawback here would be the dependency on System.Buffers.
  • Loading branch information
@jonathanpeppers
jonathanpeppers committed Feb 21, 2020
1 parent ef61e97 commit 750c780
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 24 deletions.
14 changes: 8 additions & 6 deletions Utilities.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
using System;
using System.Collections.Generic;
using System.Buffers;
using System.IO;
using System.Text;
using System.Runtime.InteropServices;

using Mono.Unix.Native;

namespace Xamarin.Tools.Zip
{
partial class Utilities
Expand Down Expand Up @@ -149,9 +147,13 @@ public unsafe static string Utf8StringPtrToString (IntPtr utf8Str)
if (len == 0)
return String.Empty;

var bytes = new byte[len];
Marshal.Copy (utf8Str, bytes, 0, len);
return Encoding.UTF8.GetString (bytes);
var bytes = ArrayPool<byte>.Shared.Rent (len);
try {
Marshal.Copy (utf8Str, bytes, 0, len);
return Encoding.UTF8.GetString (bytes, 0, len);
} finally {
ArrayPool<byte>.Shared.Return (bytes);
}
}

public static void FreeUtf8StringPtr (IntPtr ptr)
Expand Down
28 changes: 18 additions & 10 deletions ZipArchive.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
using System;
using System.Buffers;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
using System.Linq;

namespace Xamarin.Tools.Zip
{
Expand Down Expand Up @@ -724,6 +724,7 @@ internal ZipException GetErrorException ()
internal static unsafe Int64 stream_callback (IntPtr state, IntPtr data, UInt64 len, SourceCommand cmd)
{
byte [] buffer = null;
int length = (int)len;
var handle = GCHandle.FromIntPtr (state);
if (!handle.IsAllocated)
return -1;
Expand All @@ -746,10 +747,14 @@ internal static unsafe Int64 stream_callback (IntPtr state, IntPtr data, UInt64
return (Int64)stream.Position;

case SourceCommand.Write:
buffer = new byte [len];
Marshal.Copy (data, buffer, 0, (int)len);
stream.Write (buffer, 0, buffer.Length);
return buffer.Length;
buffer = ArrayPool<byte>.Shared.Rent (length);
try {
Marshal.Copy (data, buffer, 0, length);
stream.Write (buffer, 0, length);
return buffer.Length;
} finally {
ArrayPool<byte>.Shared.Return (buffer);
}

case SourceCommand.SeekWrite:
case SourceCommand.Seek:
Expand All @@ -763,14 +768,17 @@ internal static unsafe Int64 stream_callback (IntPtr state, IntPtr data, UInt64
break;

case SourceCommand.Read:
var length = (int)len;
if (length > stream.Length - stream.Position) {
length = (int)(stream.Length - stream.Position);
}
buffer = new byte [length];
int bytesRead = stream.Read (buffer, 0, length);
Marshal.Copy (buffer, 0, data, bytesRead);
return bytesRead;
buffer = ArrayPool<byte>.Shared.Rent (length);
try {
int bytesRead = stream.Read (buffer, 0, length);
Marshal.Copy (buffer, 0, data, bytesRead);
return bytesRead;
} finally {
ArrayPool<byte>.Shared.Return (buffer);
}

case SourceCommand.BeginWrite:
case SourceCommand.Open:
Expand Down
19 changes: 11 additions & 8 deletions ZipEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
using System;
using System.Buffers;
using System.Collections.Generic;
using System.IO;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -386,14 +387,16 @@ void DoExtract (IntPtr zipFile, string destinationPath, FileMode outputFileMode,

void DoExtract (IntPtr zipFile, Stream destinationStream, EntryExtractEventArgs args)
{
var buf = new byte [ReadBufSize];

long nread;

while ((nread = Native.zip_fread (zipFile, buf, (ulong)buf.Length)) > 0) {
destinationStream.Write (buf, 0, (int)nread);
args.ProcessedSoFar += (ulong)nread;
OnExtract (args);
var buf = ArrayPool<byte>.Shared.Rent (ReadBufSize);
try {
long nread;
while ((nread = Native.zip_fread (zipFile, buf, (ulong)buf.Length)) > 0) {
destinationStream.Write (buf, 0, (int)nread);
args.ProcessedSoFar += (ulong)nread;
OnExtract (args);
}
} finally {
ArrayPool<byte>.Shared.Return (buf);
}
}

Expand Down
1 change: 1 addition & 0 deletions libZipSharp.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,6 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Mono.Posix.NETStandard" Version="1.0.0" PrivateAssets="none" />
<PackageReference Include="System.Buffers" Version="4.5.0" />
</ItemGroup>
</Project>

0 comments on commit 750c780

Please sign in to comment.