Skip to content

Commit

Permalink
Improved performance of the ToByteArray methods (#1447)
Browse files Browse the repository at this point in the history
  • Loading branch information
dlemstra committed Oct 16, 2023
1 parent 58fbcd1 commit d284055
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 142 deletions.
108 changes: 33 additions & 75 deletions src/Magick.NET/Helpers/ByteArrayWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,73 +3,53 @@

using System;
using System.IO;
using System.Runtime.InteropServices;
using ImageMagick.Helpers;

namespace ImageMagick;

internal sealed unsafe class ByteArrayWrapper : IDisposable
internal sealed unsafe class ByteArrayWrapper
{
internal const int BufferSize = 81920;

private readonly byte[] _buffer;
private readonly byte* _bufferStart;
private readonly GCHandle _handle;
private int _bufferOffset;
private byte[] _bytes;
private int _offset;

public ByteArrayWrapper()
{
_buffer = new byte[BufferSize];
_handle = GCHandle.Alloc(_buffer, GCHandleType.Pinned);
_bufferStart = (byte*)_handle.AddrOfPinnedObject().ToPointer();
_bufferOffset = 0;
_bytes = new byte[0];
_offset = 0;
}
private byte[] _bytes = new byte[8192];
private int _offset = 0;
private int _length = 0;

public byte[] GetBytes()
{
AppendBufferToBytes();
ResizeBytes(_length);
return _bytes;
}

public void Dispose()
=> _handle.Free();

public long Read(IntPtr data, UIntPtr count, IntPtr user_data)
{
var total = (int)count;
if (total == 0)
if (data == IntPtr.Zero)
return 0;

if (data == IntPtr.Zero)
var total = (int)count;
if (total == 0)
return 0;

var length = Math.Min(total, _bytes.Length - _offset);
var length = Math.Min(total, _length - _offset);
if (length != 0)
{
var destination = (byte*)data.ToPointer();
fixed (byte* source = _bytes)
{
NativeMemory.Copy(source + _offset, destination, length);
_offset += length;
}

_offset += length;
}

return length;
}

public long Seek(long offset, IntPtr whence, IntPtr user_data)
{
AppendBufferToBytes();

var newOffset = (int)((SeekOrigin)whence switch
{
SeekOrigin.Begin => offset,
SeekOrigin.Current => _offset + offset,
SeekOrigin.End => _bytes.Length - offset,
SeekOrigin.End => _length - offset,
_ => -1,
});

Expand All @@ -78,74 +58,52 @@ public long Seek(long offset, IntPtr whence, IntPtr user_data)

if (newOffset < 0)
return -1;
else if (newOffset > _bytes.Length)
ResizeBytes(newOffset);

_offset = newOffset;

return _offset;
}

public long Tell(IntPtr user_data)
=> _offset + _bufferOffset;
=> _offset;

public long Write(IntPtr data, UIntPtr count, IntPtr user_data)
{
var total = (int)count;
if (total == 0)
return 0;

if (data == IntPtr.Zero)
return 0;

var source = (byte*)data.ToPointer();

while (total > 0)
{
var length = Math.Min(total, BufferSize - _bufferOffset);
if (_offset < _bytes.Length)
{
AppendBufferToBytes();
var total = (int)count;

length = Math.Min(length, _bytes.Length - _offset);
source = FillBuffer(source, length);
if (total == 0)
return 0;

CopyBufferToBytes();
}
else
{
source = FillBuffer(source, length);
var newOffset = _offset + total;

if (_bufferOffset == BufferSize)
AppendBufferToBytes();
}
EnsureLength(newOffset);

total -= length;
var source = (byte*)data.ToPointer();
fixed (byte* destination = _bytes)
{
NativeMemory.Copy(source, destination + _offset, total);
}

return (long)count;
_offset = newOffset;

return total;
}

private void AppendBufferToBytes()
private void EnsureLength(int length)
{
if (_bufferOffset == 0)
if (length < _length)
return;

ResizeBytes(_bytes.Length + _bufferOffset);
CopyBufferToBytes();
}
_length = length;

private void CopyBufferToBytes()
{
Buffer.BlockCopy(_buffer, 0, _bytes, _offset, _bufferOffset);
_offset += _bufferOffset;
_bufferOffset = 0;
}
if (_length < _bytes.Length)
return;

private byte* FillBuffer(byte* source, int length)
{
NativeMemory.Copy(source, _bufferStart + _bufferOffset, length);
_bufferOffset += length;
return source + length;
var newLength = Math.Max(_bytes.Length * 2, _length);
ResizeBytes(newLength);
}

private void ResizeBytes(int length)
Expand Down
2 changes: 1 addition & 1 deletion src/Magick.NET/MagickImage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6535,7 +6535,7 @@ public byte[] ToByteArray()
{
_settings.FileName = null;

using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();
var writer = new ReadWriteStreamDelegate(wrapper.Write);
var seeker = new SeekStreamDelegate(wrapper.Seek);
var teller = new TellStreamDelegate(wrapper.Tell);
Expand Down
2 changes: 1 addition & 1 deletion src/Magick.NET/MagickImageCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1466,7 +1466,7 @@ public byte[] ToByteArray()
{
AttachImages();

using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();
var writer = new ReadWriteStreamDelegate(wrapper.Write);
var seeker = new SeekStreamDelegate(wrapper.Seek);
var teller = new TellStreamDelegate(wrapper.Tell);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright Dirk Lemstra https://github.com/dlemstra/Magick.NET.
// Licensed under the Apache License, Version 2.0.

using System;
using System.IO;
using ImageMagick;
using Xunit;

namespace Magick.NET.Tests;

public partial class ByteArrayWrapperTests
{
public class TheGetBytesMethod
{
[Fact]
public unsafe void ShouldOnlyReturnTheWrittenBytes()
{
var wrapper = new ByteArrayWrapper();
wrapper.Seek(42, (IntPtr)SeekOrigin.Current, IntPtr.Zero);
wrapper.Seek(0, (IntPtr)SeekOrigin.Begin, IntPtr.Zero);

var buffer = new byte[5];
fixed (byte* p = buffer)
{
wrapper.Write((IntPtr)p, (UIntPtr)3, IntPtr.Zero);
}

var bytes = wrapper.GetBytes();
Assert.Equal(3, bytes.Length);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class TheReadMethod
[Fact]
public void ShouldReturnZeroWhenBufferIsNull()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();

var count = wrapper.Read(IntPtr.Zero, (UIntPtr)10, IntPtr.Zero);
Assert.Equal(0, count);
Expand All @@ -24,7 +24,7 @@ public void ShouldReturnZeroWhenBufferIsNull()
[Fact]
public unsafe void ShouldReturnZeroWhenNothingShouldBeRead()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();

var buffer = new byte[255];
fixed (byte* p = buffer)
Expand All @@ -37,27 +37,28 @@ public unsafe void ShouldReturnZeroWhenNothingShouldBeRead()
[Fact]
public unsafe void ShouldReturnTheNumberOfBytesThatCouldBeRead()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();

var offset = 0L;
var buffer = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
fixed (byte* p = buffer)
{
wrapper.Write((IntPtr)p, (UIntPtr)buffer.Length, IntPtr.Zero);
var count = wrapper.Write((IntPtr)p, (UIntPtr)buffer.Length, IntPtr.Zero);
Assert.Equal(10, count);

wrapper.Seek(0, (IntPtr)SeekOrigin.Current, IntPtr.Zero);
offset = wrapper.Seek(0, (IntPtr)SeekOrigin.Current, IntPtr.Zero);
Assert.Equal(10, offset);

wrapper.Write((IntPtr)p, (UIntPtr)buffer.Length, IntPtr.Zero);
count = wrapper.Write((IntPtr)p, (UIntPtr)buffer.Length, IntPtr.Zero);
Assert.Equal(10, count);
}

offset = wrapper.Seek(15, (IntPtr)SeekOrigin.Begin, IntPtr.Zero);
Assert.Equal(15, offset);

buffer = new byte[] { 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 };
fixed (byte* p = buffer)
{
wrapper.Seek(0, (IntPtr)SeekOrigin.Begin, IntPtr.Zero);

wrapper.Write((IntPtr)p, (UIntPtr)buffer.Length, IntPtr.Zero);

wrapper.Seek(5, (IntPtr)SeekOrigin.Current, IntPtr.Zero);

var count = wrapper.Read((IntPtr)p, (UIntPtr)10, IntPtr.Zero);
Assert.Equal(5, count);
Assert.Equal(20, wrapper.Tell(IntPtr.Zero));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,27 @@ public class TheSeekMethod
[Fact]
public unsafe void ShouldReturnTheNewOffset()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();

var result = wrapper.Seek(10, (IntPtr)SeekOrigin.Begin, IntPtr.Zero);

Assert.Equal(10, result);
Assert.Equal(10, wrapper.GetBytes().Length);
}

[Fact]
public unsafe void ShouldReturnTheNewOffsetSStartingFromTheCurrentPosition()
public unsafe void ShouldNotChangeTheSizeOfTheBytes()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();

var result = wrapper.Seek(10, (IntPtr)SeekOrigin.Begin, IntPtr.Zero);

Assert.Empty(wrapper.GetBytes());
}

[Fact]
public unsafe void ShouldReturnTheNewOffsetStartingFromTheCurrentPosition()
{
var wrapper = new ByteArrayWrapper();

var buffer = new byte[42];
fixed (byte* p = buffer)
Expand All @@ -42,7 +51,7 @@ public unsafe void ShouldReturnTheNewOffsetSStartingFromTheCurrentPosition()
[Fact]
public unsafe void ShouldReturnCorrectValueFromEnd()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();

var buffer = new byte[42];
fixed (byte* p = buffer)
Expand All @@ -57,7 +66,7 @@ public unsafe void ShouldReturnCorrectValueFromEnd()
[Fact]
public void ShouldReturnMinusOneForInvalidOffset()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();
var result = wrapper.Seek(-10, (IntPtr)SeekOrigin.Current, IntPtr.Zero);

Assert.Equal(-1, result);
Expand All @@ -66,7 +75,7 @@ public void ShouldReturnMinusOneForInvalidOffset()
[Fact]
public void ShouldNotChangeOffsetWhenValueIsInvalid()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();
wrapper.Seek(-10, (IntPtr)SeekOrigin.Current, IntPtr.Zero);

Assert.Equal(0, wrapper.Tell(IntPtr.Zero));
Expand All @@ -75,7 +84,7 @@ public void ShouldNotChangeOffsetWhenValueIsInvalid()
[Fact]
public void ShouldReturnMinusOneForInvalidWhence()
{
using var wrapper = new ByteArrayWrapper();
var wrapper = new ByteArrayWrapper();
var result = wrapper.Seek(0, (IntPtr)3, IntPtr.Zero);

Assert.Equal(-1, result);
Expand Down
Loading

0 comments on commit d284055

Please sign in to comment.