Azure Blob storage: pooled reads, streaming serializer, buffered writes#9879
Azure Blob storage: pooled reads, streaming serializer, buffered writes#9879egil wants to merge 2 commits intodotnet:mainfrom
Conversation
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs
Outdated
Show resolved
Hide resolved
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
from
7a23245 to
7ec8b60
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in performance optimization for Azure Blob Storage reads by using pooled buffers to reduce memory pressure on the Large Object Heap (LOH) and minimize Gen2 garbage collection. The change adds a new UsePooledBufferForReads option to AzureBlobStorageOptions that, when enabled, switches from DownloadContentAsync to DownloadStreamingAsync with ArrayPool-rented buffers.
Changes:
- Added
UsePooledBufferForReadsoption toAzureBlobStorageOptionsfor opt-in buffer pooling - Modified
ReadStateAsyncto use streaming download with pooled buffers when the option is enabled - Added comprehensive test coverage with
PersistenceGrainTests_AzureBlobStore_PooledReads - Added performance benchmark
AzureBlobReadStateBenchmarkdemonstrating 56-74% memory allocation reduction for large payloads
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorageOptions.cs | Adds new UsePooledBufferForReads boolean option with documentation warning about buffer retention |
| src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs | Implements pooled buffer logic using DownloadStreamingAsync and ArrayPool<byte>.Shared when option is enabled |
| test/Extensions/TesterAzureUtils/Persistence/PersistenceGrainTests_AzureBlobStore_PooledReads.cs | New test class verifying functionality with pooled reads enabled |
| test/Benchmarks/GrainStorage/AzureBlobReadStateBenchmark.cs | New benchmark comparing pooled vs non-pooled read performance |
| test/Benchmarks/Program.cs | Adds benchmark runner entry for the new Azure Blob read state benchmark |
| test/Benchmarks/run_test.cmd | Updates script to run the new benchmark |
| test/Benchmarks/Properties/launchSettings.json | Removes hardcoded launch profile |
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs
Outdated
Show resolved
Hide resolved
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs
Outdated
Show resolved
Hide resolved
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
from
7ec8b60 to
99a2f67
Compare
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs
Outdated
Show resolved
Hide resolved
|
I assume most of the remaining allocations are from the actual state object being loaded. Does that sound right? I also see an eTag string + the BinaryData object + the async state machine, but those are not worth going after at this point. |
That is my assumption too.
Agreed. First stab at this is to reduce GC churn. Similar pattern may be applicable to other storage providers though, but I do feel like blob storage is more likely to have larger payloads than e.g. a SQL server. Long term it would be great to do away with byte[] entirely and pass a stream or similar abstraction directly to the grain storage serializer, but that requires changing API surface. |
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
from
99a2f67 to
b8fb3bc
Compare
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorageOptions.cs
Show resolved
Hide resolved
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorageOptions.cs
Show resolved
Hide resolved
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs
Outdated
Show resolved
Hide resolved
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
2 times, most recently
from
e4321a9 to
370139e
Compare
egil
changed the title
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
3 times, most recently
from
cee520a to
7793d64
Compare
|
|
|
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorage.cs
Outdated
Show resolved
Hide resolved
src/Orleans.Core/Providers/StorageSerializer/OrleansGrainStateSerializer.cs
Show resolved
Hide resolved
src/Azure/Orleans.Persistence.AzureStorage/Providers/Storage/AzureBlobStorageOptions.cs
Show resolved
Hide resolved
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
from
7793d64 to
59a6eac
Compare
|
Not sure why this test is failing, and if it has anything to do with the changes on this PR: |
| /// <summary> | ||
| /// Optional stream-based serializer for grain state. | ||
| /// </summary> | ||
| public interface IGrainStorageStreamingSerializer |
There was a problem hiding this comment.
@ReubenBond said on Discord he prefers something like this for the abstraction.
/// <summary>
/// Optional stream-based serializer for grain state.
/// </summary>
public interface IGrainStorageStreamingSerializer
{
/// <summary>
/// Serializes the object input to a stream.
/// </summary>
/// <param name="input">The object to serialize.</param>
/// <param name="destination">The destination buffer writer.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <typeparam name="T">The input type.</typeparam>
ValueTask SerializeAsync<T>(T input, IBufferWriter<byte> destination, CancellationToken cancellationToken = default);
/// <summary>
/// Deserializes the provided data from a stream.
/// </summary>
/// <param name="input">The input byte sequence.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <typeparam name="T">The output type.</typeparam>
/// <returns>The deserialized object, or null.</returns>
ValueTask<T?> DeserializeAsync<T>(ReadOnlySequence<byte> input, CancellationToken cancellationToken = default);
}For the data providers where Stream is native, we can then include helper extensions method that map to/from IBufferWriter<byte> and ReadOnlySequence<byte>
There was a problem hiding this comment.
For serialization, we can use ready build adapters in Orleans:
public static class GrainStorageStreamingSerializerExtensions
{
/// <summary>
/// Serializes the object input to a stream.
/// </summary>
/// <param name="input">The object to serialize.</param>
/// <param name="destination">The destination stream.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <typeparam name="T">The input type.</typeparam>
public static ValueTask SerializeAsync<T>(this IGrainStorageStreamingSerializer serializer, T input, Stream destination, CancellationToken cancellationToken = default)
{
ArgumentNullException.ThrowIfNull(serializer);
ArgumentNullException.ThrowIfNull(destination);
if (destination is MemoryStream memoryStream)
{
return serializer.SerializeAsync(input, new MemoryStreamBufferWriter(memoryStream), cancellationToken);
}
else
{
return serializer.SerializeAsync(input, new ArrayStreamBufferWriter(destination), cancellationToken);
}
}
}For DeserializeAsync, I would love a suggestion for how to adapt a Stream to ReadOnlySequence<byte> without loading all data into memory first. Suggestions?
There was a problem hiding this comment.
The naming feels off, now you've switched from stream, maybe change to IGrainStateBufferSerializer or something.
My gut is telling me, drop the ValueTask, by this point all IO should be done, and Task/Await is just overhead 99% of the time, unless I am missing why you would need...
There was a problem hiding this comment.
The naming feels off, now you've switched from stream, maybe change to
IGrainStateBufferSerializeror something.My gut is telling to drop the ValueTask, by this point all IO should be done, and Task/Await is just overhead 99% of the time, unless I am missing why you would need...
I hope IO is not done at this point, no, since that means data has been loaded into memory, which was the main problem I was trying to avoid. With large datasets, e.g., blobs, that leads to more GC churn. So there need to be support for streaming data from blob storage to the serializer and then to objects.
Calling the interface IGrainStateBufferSerializer is fine though, no objections there.
There was a problem hiding this comment.
I was thinking about the IO and buffering in the serializer implementations (JsonGrainStorageSerializer, OrleansGrainStorageSerializer) and how it flows into AzureBlobGrainStorage.
Note: overhead here is tiny — pure micro-optimization territory!
The overall solution I was getting at earlier on Discord before I dropped off 😅 has been implemented at the calling level (in the provider) rather than inside the serializer.
// In AzureBlobGrainStorage.UploadSerializedStateBufferedAsync<T>
var bufferStream = PooledBufferStream.Rent();
try
{
// Serialize: sync write to the pooled stream (no real await needed inside most impls)
await streamSerializer.SerializeAsync(value, bufferStream).ConfigureAwait(false);
bufferStream.Position = 0;
// Actual IO: upload from the pooled stream
return await blob.UploadAsync(bufferStream, options).ConfigureAwait(false);
}
finally
{
PooledBufferStream.Return(bufferStream);
}In JsonGrainStorageSerializer.SerializeAsync (similar for Orleans serializer):
public ValueTask SerializeAsync<T>(T value, Stream destination, CancellationToken ct = default)
{
ct.ThrowIfCancellationRequested();
_orleansJsonSerializer.Serialize(value, typeof(T), destination); // sync write
return ValueTask.CompletedTask; // no suspension
}The await on serialize is still basically zero-cost (completed ValueTask), but unnecessary.
There was a problem hiding this comment.
Ahh I see. If there is no need for asynchrony in the serializer implementations, then dropping ValueTask is fine. Looks like there are no asynchronous methods on ReadOnlySequence nor on IBufferWriter, so probably a good indicator ValueTask is not needed?
| public bool UsePooledBufferForReads { get; set; } = true; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the write path to use when a stream serializer is available. |
Introduce IGrainStorageStreamingSerializer and stream overloads for OrleansJsonSerializer plus Orleans/Json grain storage serializers. Azure blob storage now supports pooled read buffers, a buffered stream write mode, and logs a warning when large payloads force a fallback to DownloadContentAsync. Add Azure Blob storage tests for pooled reads and streaming serializer behavior, and add focused grain storage benchmarks (binary vs streaming) across Orleans, Newtonsoft.Json, and STJ. Alternatives considered: full streaming OpenWriteAsync with separate ETag readback (rejected due to concurrency race), and IBufferWriter/ReadOnlySequence buffer paths (explored, but didnt improve performance or allocation in a meaningful way, so but dropped for now). Buffered stream uploads and pooled reads were kept as the best allocation/compatibility tradeoff while keeping BinaryData writes as the default.
…zeAsync return type
egil
force-pushed
the
pooled-azure-blob-storage-read
branch
from
77c0d03 to
485d518
Compare
Summary
IGrainStorageStreamingSerializerplus stream overloads forOrleansJsonSerializer; Orleans and Newtonsoft.Json grain storage serializers implement it.DownloadStreamingAsync, and supports a buffered stream write mode using pooled segments; default write path remainsBinaryData.int.MaxValue.Tests/Benchmarks
Experiments/Decisions
OpenWriteAsyncfor fully streaming uploads; rejected due to an ETag/concurrency race whenGetPropertiesAsyncis needed post-upload.IBufferWriter/ReadOnlySequenceserializer paths; dropped to keep the API surface smaller and because Blob SDK paths are stream-first.BinaryDataas the default for throughput.Ill post measurements from my laptop in comments below.