-
-
Notifications
You must be signed in to change notification settings - Fork 888
Reduced intermediate allocations #2415
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
Changes from all commits
b099bda
d9e8d79
31424c0
858a848
1d3ae0e
3c3479e
b9b6f72
5d65ef0
57d0793
7b2923d
1e96db9
53fa6da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals | |
/// <summary> | ||
/// The temp buffer used to reduce allocations. | ||
/// </summary> | ||
private readonly byte[] buffer = new byte[16]; | ||
private ScratchBuffer buffer; // mutable struct, don't make readonly | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we use scratch buffers in a few places. |
||
|
||
/// <summary> | ||
/// The global color table. | ||
|
@@ -249,13 +249,13 @@ public ImageInfo Identify(BufferedReadStream stream, CancellationToken cancellat | |
/// <param name="stream">The <see cref="BufferedReadStream"/> containing image data.</param> | ||
private void ReadGraphicalControlExtension(BufferedReadStream stream) | ||
{ | ||
int bytesRead = stream.Read(this.buffer, 0, 6); | ||
int bytesRead = stream.Read(this.buffer.Span, 0, 6); | ||
if (bytesRead != 6) | ||
{ | ||
GifThrowHelper.ThrowInvalidImageContentException("Not enough data to read the graphic control extension"); | ||
} | ||
|
||
this.graphicsControlExtension = GifGraphicControlExtension.Parse(this.buffer); | ||
this.graphicsControlExtension = GifGraphicControlExtension.Parse(this.buffer.Span); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -264,13 +264,13 @@ private void ReadGraphicalControlExtension(BufferedReadStream stream) | |
/// <param name="stream">The <see cref="BufferedReadStream"/> containing image data.</param> | ||
private void ReadImageDescriptor(BufferedReadStream stream) | ||
{ | ||
int bytesRead = stream.Read(this.buffer, 0, 9); | ||
int bytesRead = stream.Read(this.buffer.Span, 0, 9); | ||
if (bytesRead != 9) | ||
{ | ||
GifThrowHelper.ThrowInvalidImageContentException("Not enough data to read the image descriptor"); | ||
} | ||
|
||
this.imageDescriptor = GifImageDescriptor.Parse(this.buffer); | ||
this.imageDescriptor = GifImageDescriptor.Parse(this.buffer.Span); | ||
if (this.imageDescriptor.Height == 0 || this.imageDescriptor.Width == 0) | ||
{ | ||
GifThrowHelper.ThrowInvalidImageContentException("Width or height should not be 0"); | ||
|
@@ -283,13 +283,13 @@ private void ReadImageDescriptor(BufferedReadStream stream) | |
/// <param name="stream">The <see cref="BufferedReadStream"/> containing image data.</param> | ||
private void ReadLogicalScreenDescriptor(BufferedReadStream stream) | ||
{ | ||
int bytesRead = stream.Read(this.buffer, 0, 7); | ||
int bytesRead = stream.Read(this.buffer.Span, 0, 7); | ||
if (bytesRead != 7) | ||
{ | ||
GifThrowHelper.ThrowInvalidImageContentException("Not enough data to read the logical screen descriptor"); | ||
} | ||
|
||
this.logicalScreenDescriptor = GifLogicalScreenDescriptor.Parse(this.buffer); | ||
this.logicalScreenDescriptor = GifLogicalScreenDescriptor.Parse(this.buffer.Span); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -306,8 +306,8 @@ private void ReadApplicationExtension(BufferedReadStream stream) | |
long position = stream.Position; | ||
if (appLength == GifConstants.ApplicationBlockSize) | ||
{ | ||
stream.Read(this.buffer, 0, GifConstants.ApplicationBlockSize); | ||
bool isXmp = this.buffer.AsSpan().StartsWith(GifConstants.XmpApplicationIdentificationBytes); | ||
stream.Read(this.buffer.Span, 0, GifConstants.ApplicationBlockSize); | ||
bool isXmp = this.buffer.Span.StartsWith(GifConstants.XmpApplicationIdentificationBytes); | ||
if (isXmp && !this.skipMetadata) | ||
{ | ||
GifXmpApplicationExtension extension = GifXmpApplicationExtension.Read(stream, this.memoryAllocator); | ||
|
@@ -331,8 +331,8 @@ private void ReadApplicationExtension(BufferedReadStream stream) | |
// http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension | ||
if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize) | ||
{ | ||
stream.Read(this.buffer, 0, GifConstants.NetscapeLoopingSubBlockSize); | ||
this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.AsSpan(1)).RepeatCount; | ||
stream.Read(this.buffer.Span, 0, GifConstants.NetscapeLoopingSubBlockSize); | ||
this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span.Slice(1)).RepeatCount; | ||
stream.Skip(1); // Skip the terminator. | ||
return; | ||
} | ||
|
@@ -762,4 +762,12 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(BufferedReadStream s | |
} | ||
} | ||
} | ||
|
||
private unsafe struct ScratchBuffer | ||
{ | ||
private const int Size = 16; | ||
private fixed byte scratch[Size]; | ||
|
||
public Span<byte> Span => MemoryMarshal.CreateSpan(ref this.scratch[0], Size); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,11 +28,6 @@ internal sealed class GifEncoderCore : IImageEncoderInternals | |
/// </summary> | ||
private readonly Configuration configuration; | ||
|
||
/// <summary> | ||
/// A reusable buffer used to reduce allocations. | ||
/// </summary> | ||
private readonly byte[] buffer = new byte[20]; | ||
|
||
/// <summary> | ||
/// Whether to skip metadata during encode. | ||
/// </summary> | ||
|
@@ -324,9 +319,10 @@ private void WriteLogicalScreenDescriptor( | |
backgroundColorIndex: unchecked((byte)transparencyIndex), | ||
ratio); | ||
|
||
descriptor.WriteTo(this.buffer); | ||
Span<byte> buffer = stackalloc byte[20]; | ||
descriptor.WriteTo(buffer); | ||
|
||
stream.Write(this.buffer, 0, GifLogicalScreenDescriptor.Size); | ||
stream.Write(buffer, 0, GifLogicalScreenDescriptor.Size); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -365,12 +361,14 @@ private void WriteComments(GifMetadata metadata, Stream stream) | |
return; | ||
} | ||
|
||
Span<byte> buffer = stackalloc byte[2]; | ||
|
||
for (int i = 0; i < metadata.Comments.Count; i++) | ||
{ | ||
string comment = metadata.Comments[i]; | ||
this.buffer[0] = GifConstants.ExtensionIntroducer; | ||
this.buffer[1] = GifConstants.CommentLabel; | ||
stream.Write(this.buffer, 0, 2); | ||
buffer[1] = GifConstants.CommentLabel; | ||
buffer[0] = GifConstants.ExtensionIntroducer; | ||
Comment on lines
+369
to
+370
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll write below a comment on why these are flipped. |
||
stream.Write(buffer); | ||
|
||
// Comment will be stored in chunks of 255 bytes, if it exceeds this size. | ||
ReadOnlySpan<char> commentSpan = comment.AsSpan(); | ||
|
@@ -437,22 +435,23 @@ private void WriteGraphicalControlExtension(GifFrameMetadata metadata, int trans | |
private void WriteExtension<TGifExtension>(TGifExtension extension, Stream stream) | ||
where TGifExtension : struct, IGifExtension | ||
{ | ||
IMemoryOwner<byte>? owner = null; | ||
Span<byte> extensionBuffer; | ||
int extensionSize = extension.ContentLength; | ||
|
||
if (extensionSize == 0) | ||
{ | ||
return; | ||
} | ||
else if (extensionSize > this.buffer.Length - 3) | ||
|
||
IMemoryOwner<byte>? owner = null; | ||
Span<byte> extensionBuffer = stackalloc byte[0]; // workaround compiler limitation | ||
if (extensionSize > 128) | ||
{ | ||
owner = this.memoryAllocator.Allocate<byte>(extensionSize + 3); | ||
extensionBuffer = owner.GetSpan(); | ||
} | ||
else | ||
{ | ||
extensionBuffer = this.buffer; | ||
extensionBuffer = stackalloc byte[extensionSize + 3]; | ||
} | ||
|
||
extensionBuffer[0] = GifConstants.ExtensionIntroducer; | ||
|
@@ -489,9 +488,10 @@ private void WriteImageDescriptor<TPixel>(ImageFrame<TPixel> image, bool hasColo | |
height: (ushort)image.Height, | ||
packed: packedValue); | ||
|
||
descriptor.WriteTo(this.buffer); | ||
Span<byte> buffer = stackalloc byte[20]; | ||
descriptor.WriteTo(buffer); | ||
|
||
stream.Write(this.buffer, 0, GifImageDescriptor.Size); | ||
stream.Write(buffer, 0, GifImageDescriptor.Size); | ||
} | ||
|
||
/// <summary> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just because I'm paranoid of overflow 😉.