-
Notifications
You must be signed in to change notification settings - Fork 5
Optimize large buffer allocations with ArrayPool #78
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 2 commits
375f042
81a3dd6
b9e4bd0
fbe7c1d
c0c62e9
a132bff
d57680f
65c8a88
f2b8c03
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 |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| // ReSharper disable PartialTypeWithSinglePart | ||
|
|
||
| using System; | ||
| using System.Buffers; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| [ExcludeFromCodeCoverage] | ||
|
|
@@ -48,17 +49,23 @@ static int GetHexValue(char c) | |
| // https://learn.microsoft.com/dotnet/api/system.convert.tohexstring#system-convert-tohexstring(system-byte()-system-int32-system-int32) | ||
| public static string ToHexString(byte[] value, int startIndex, int length) | ||
| { | ||
| var c = new char[length * 2]; | ||
| var c = ArrayPool<char>.Shared.Rent(length * 2); | ||
|
Owner
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. Let's rename this to var chars
Contributor
Author
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. Done in a132bff - renamed |
||
| try | ||
| { | ||
| for (var i = 0; i < length; i++) | ||
| { | ||
| var b = value[startIndex + i] >> 4; | ||
| c[i * 2] = (char)(55 + b + (((b - 10) >> 31) & -7)); | ||
| b = value[startIndex + i] & 0xF; | ||
| c[i * 2 + 1] = (char)(55 + b + (((b - 10) >> 31) & -7)); | ||
| } | ||
|
|
||
| for (var i = 0; i < length; i++) | ||
| return new string(c, 0, length * 2); | ||
| } | ||
| finally | ||
| { | ||
| var b = value[startIndex + i] >> 4; | ||
| c[i * 2] = (char)(55 + b + (((b - 10) >> 31) & -7)); | ||
| b = value[startIndex + i] & 0xF; | ||
| c[i * 2 + 1] = (char)(55 + b + (((b - 10) >> 31) & -7)); | ||
| ArrayPool<char>.Shared.Return(c); | ||
| } | ||
|
|
||
| return new string(c); | ||
| } | ||
|
|
||
| // https://learn.microsoft.com/dotnet/api/system.convert.tohexstring#system-convert-tohexstring(system-byte()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| // ReSharper disable PartialTypeWithSinglePart | ||
|
|
||
| using System; | ||
| using System.Buffers; | ||
| using System.IO; | ||
| using System.Text; | ||
| using System.Threading; | ||
|
|
@@ -30,21 +31,27 @@ internal static class MemberPolyfills_Net70_TextReader | |
| public async Task<string> ReadToEndAsync(CancellationToken cancellationToken) | ||
| { | ||
| var result = new StringBuilder(); | ||
| var buffer = new char[4096]; | ||
|
|
||
| while (true) | ||
| var buffer = ArrayPool<char>.Shared.Rent(4096); | ||
|
Owner
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. Maybe can use MemoryPool here, no? And in other places where async methods that support Memory are used.
Contributor
Author
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. Done in a132bff - switched to MemoryPool for cleaner disposal pattern. Note: StringBuilder doesn't support Span/Memory append in older frameworks, so we convert the Memory slice to string (which allocates, but is necessary for compatibility). |
||
| try | ||
| { | ||
| var charsRead = await reader | ||
| .ReadAsync(buffer, cancellationToken) | ||
| .ConfigureAwait(false); | ||
| while (true) | ||
| { | ||
| var charsRead = await reader | ||
| .ReadAsync(buffer, cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (charsRead <= 0) | ||
| break; | ||
| if (charsRead <= 0) | ||
| break; | ||
|
|
||
| result.Append(buffer, 0, charsRead); | ||
| } | ||
| result.Append(buffer, 0, charsRead); | ||
| } | ||
|
|
||
| return result.ToString(); | ||
| return result.ToString(); | ||
| } | ||
| finally | ||
| { | ||
| ArrayPool<char>.Shared.Return(buffer); | ||
| } | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||
| // ReSharper disable PartialTypeWithSinglePart | ||||||
|
|
||||||
| using System; | ||||||
| using System.Buffers; | ||||||
| using System.Text; | ||||||
| using System.Diagnostics.CodeAnalysis; | ||||||
| using System.Security.Cryptography; | ||||||
|
|
@@ -46,11 +47,21 @@ public static void Shuffle<T>(Span<T> items) | |||||
| // https://learn.microsoft.com/dotnet/api/system.security.cryptography.randomnumbergenerator.gethexstring#system-security-cryptography-randomnumbergenerator-gethexstring(system-int32-system-boolean) | ||||||
| public static string GetHexString(int stringLength, bool lowercase = false) | ||||||
| { | ||||||
| var bytes = new byte[(stringLength + 1) / 2]; | ||||||
| RandomNumberGenerator.Fill(bytes); | ||||||
| var byteCount = (stringLength + 1) / 2; | ||||||
|
Owner
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. Can be inlined?
Contributor
Author
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. Done in a132bff - inlined |
||||||
| var bytes = ArrayPool<byte>.Shared.Rent(byteCount); | ||||||
| try | ||||||
| { | ||||||
| RandomNumberGenerator.Fill(bytes.AsSpan(0, byteCount)); | ||||||
|
|
||||||
| var hex = lowercase ? Convert.ToHexStringLower(bytes) : Convert.ToHexString(bytes); | ||||||
| return hex.Substring(0, stringLength); | ||||||
| var hex = lowercase | ||||||
| ? Convert.ToHexStringLower(bytes, 0, byteCount) | ||||||
| : Convert.ToHexString(bytes, 0, byteCount); | ||||||
| return hex.Substring(0, stringLength); | ||||||
| } | ||||||
| finally | ||||||
| { | ||||||
| ArrayPool<byte>.Shared.Return(bytes); | ||||||
|
||||||
| ArrayPool<byte>.Shared.Return(bytes); | |
| ArrayPool<byte>.Shared.Return(bytes, clearArray: true); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| // ReSharper disable InconsistentNaming | ||
| // ReSharper disable PartialTypeWithSinglePart | ||
|
|
||
| using System.Buffers; | ||
| using System.IO; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
|
|
@@ -16,14 +17,21 @@ internal static class MemberPolyfills_NetCore10_Stream | |
| // https://learn.microsoft.com/dotnet/api/system.io.stream.copyto#system-io-stream-copyto(system-io-stream-system-int32) | ||
| public void CopyTo(Stream destination, int bufferSize) | ||
| { | ||
| var buffer = new byte[bufferSize]; | ||
| while (true) | ||
| var buffer = ArrayPool<byte>.Shared.Rent(bufferSize); | ||
| try | ||
| { | ||
| var bytesRead = source.Read(buffer, 0, buffer.Length); | ||
| if (bytesRead <= 0) | ||
| break; | ||
| while (true) | ||
| { | ||
| var bytesRead = source.Read(buffer, 0, bufferSize); | ||
|
Owner
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. Read up to buffer.Length
Contributor
Author
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. Done in d57680f - changed to read up to |
||
| if (bytesRead <= 0) | ||
| break; | ||
|
|
||
| destination.Write(buffer, 0, bytesRead); | ||
| destination.Write(buffer, 0, bytesRead); | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| ArrayPool<byte>.Shared.Return(buffer); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| // ReSharper disable InconsistentNaming | ||
| // ReSharper disable PartialTypeWithSinglePart | ||
|
|
||
| using System.Buffers; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Text; | ||
|
|
@@ -134,18 +135,24 @@ public static async Task<string> ReadAllTextAsync(string path, CancellationToken | |
| using var reader = new StreamReader(stream); | ||
|
|
||
| var content = new StringBuilder(); | ||
| var buffer = new char[4096]; | ||
| var buffer = ArrayPool<char>.Shared.Rent(4096); | ||
| try | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| int charsRead; | ||
| while ((charsRead = await reader.ReadAsync(buffer, 0, 4096).ConfigureAwait(false)) > 0) | ||
|
Owner
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. Can read into entire buffer, it's okay if there's more bytes
Contributor
Author
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. Done in a132bff - changed to read into full buffer ( |
||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| content.Append(buffer, 0, charsRead); | ||
| } | ||
|
|
||
| int charsRead; | ||
| while ((charsRead = await reader.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false)) > 0) | ||
| return content.ToString(); | ||
| } | ||
| finally | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| content.Append(buffer, 0, charsRead); | ||
| ArrayPool<char>.Shared.Return(buffer); | ||
| } | ||
|
|
||
| return content.ToString(); | ||
| } | ||
|
|
||
| // https://learn.microsoft.com/dotnet/api/system.io.file.readalltextasync#system-io-file-readalltextasync(system-string-system-text-encoding-system-threading-cancellationtoken) | ||
|
|
@@ -155,18 +162,24 @@ public static async Task<string> ReadAllTextAsync(string path, Encoding encoding | |
| using var reader = new StreamReader(stream, encoding); | ||
|
|
||
| var content = new StringBuilder(); | ||
| var buffer = new char[4096]; | ||
| var buffer = ArrayPool<char>.Shared.Rent(4096); | ||
| try | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| int charsRead; | ||
| while ((charsRead = await reader.ReadAsync(buffer, 0, 4096).ConfigureAwait(false)) > 0) | ||
|
Owner
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. Same here, it's okay if we read more bytes. Apply this logic in other relevant places
Contributor
Author
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. Done in a132bff - applied the same change to both ReadAllTextAsync overloads |
||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| content.Append(buffer, 0, charsRead); | ||
| } | ||
|
|
||
| int charsRead; | ||
| while ((charsRead = await reader.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false)) > 0) | ||
| return content.ToString(); | ||
| } | ||
| finally | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| content.Append(buffer, 0, charsRead); | ||
| ArrayPool<char>.Shared.Return(buffer); | ||
| } | ||
|
|
||
| return content.ToString(); | ||
| } | ||
|
|
||
| // https://learn.microsoft.com/dotnet/api/system.io.file.writeallbytesasync#system-io-file-writeallbytesasync(system-string-system-byte()-system-threading-cancellationtoken) | ||
|
|
||
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 can be inlined?