Add ArcBuffer support to Orleans serialization#10066
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extends Orleans’ serialization buffer plumbing to support ArcBuffer as an input source for readers and deserializers, enabling multi-page reads and round-trips directly over ArcBuffer/ArcBufferWriter.
Changes:
- Added
Reader.Create(ArcBuffer, ...)and anArcBufferReaderInputadaptor for multi-page reading/positioning/skip/fork behavior. - Added
Serializer/Serializer<T>overloads to deserialize directly fromArcBuffer. - Added
Writer.Create(ArcBufferWriter, ...)using a wrapper which prevents disposing the underlyingArcBufferWriter, plus new unit tests and testkit updates.
Show a summary per file
| File | Description |
|---|---|
| test/Orleans.Serialization.UnitTests/Buffers/ArcBufferWriterSerializationTests.cs | Adds focused tests for multi-page writer/reader and serializer round-trips using ArcBufferWriter + ArcBuffer. |
| src/Orleans.Serialization/Serializer.cs | Adds Deserialize(... ArcBuffer ...) overloads on Serializer and Serializer<T>. |
| src/Orleans.Serialization/Buffers/Writer.cs | Adds Writer.Create(ArcBufferWriter, ...) and ArcBufferWriterWrapper to avoid disposing the underlying writer. |
| src/Orleans.Serialization/Buffers/Reader.cs | Adds Reader.Create(ArcBuffer, ...) and integrates ArcBufferReaderInput into reader fast paths and navigation logic. |
| src/Orleans.Serialization/Buffers/Adaptors/BufferSliceReaderInput.cs | Introduces ArcBufferReaderInput adaptor implementation. |
| src/Orleans.Serialization.TestKit/FieldCodecTester.cs | Updates serialization test helper to produce/format/deserialize via ArcBufferWriter + ArcBuffer. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/Orleans.Serialization/Serializer.cs:889
- This new public
Deserialize(ArcBuffer source)overload (onSerializer<T>) changes the Orleans.Serialization public API surface, butsrc/api/Orleans.Serialization/Orleans.Serialization.csdoes not currently include it. Update the generated API surface file to reflect this addition.
public T Deserialize(ArcBuffer source)
{
using var session = _sessionPool.GetSession();
var reader = Reader.Create(source, session);
var field = reader.ReadFieldHeader();
return _codec.ReadValue(ref reader, field);
}
src/Orleans.Serialization/Buffers/Adaptors/BufferSliceReaderInput.cs:125
ArcBufferReaderInputis in theOrleans.Serialization.Buffers.Adaptorsnamespace, but it referencesArcBufferandArcBufferPagewhich are declared inOrleans.Serialization.Buffers. This file does not import that namespace, so these type references will not resolve and the project will fail to compile. Add ausing Orleans.Serialization.Buffers;(or fully-qualify the types) soArcBuffer/ArcBufferPageare in scope.
public struct ArcBufferReaderInput(in ArcBuffer slice)
{
private readonly ArcBuffer _slice = slice;
private ArcBufferPage _page = slice.First;
private int _position;
- Files reviewed: 6/6 changed files
- Comments generated: 5
| /// Disposing a <see cref="Writer{TBufferWriter}"/> over this wrapper does not dispose the underlying <see cref="ArcBufferWriter"/>. | ||
| /// </remarks> | ||
| /// <param name="bufferWriter">The wrapped buffer writer.</param> | ||
| public readonly struct ArcBufferWriterWrapper(ArcBufferWriter bufferWriter) : IBufferWriter<byte> | ||
| { | ||
| /// <inheritdoc/> | ||
| public void Advance(int count) => ((IBufferWriter<byte>)bufferWriter).Advance(count); | ||
|
|
||
| /// <inheritdoc/> | ||
| public Memory<byte> GetMemory(int sizeHint = 0) => bufferWriter.GetMemory(sizeHint); | ||
|
|
||
| /// <inheritdoc/> | ||
| public Span<byte> GetSpan(int sizeHint = 0) => bufferWriter.GetSpan(sizeHint); | ||
| } |
There was a problem hiding this comment.
ArcBufferWriterWrapper is a new public type, but the committed API baseline in src/api/Orleans.Serialization/Orleans.Serialization.cs does not include it. Update the generated API surface file so this new public type is represented.
| /// <summary> | ||
| /// Creates a reader for the provided buffer. | ||
| /// </summary> | ||
| /// <param name="input">The input.</param> | ||
| /// <param name="session">The session.</param> | ||
| /// <returns>A new <see cref="Reader{TInput}"/>.</returns> | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static Reader<ArcBufferReaderInput> Create(ArcBuffer input, SerializerSession session) => new(new ArcBufferReaderInput(in input), session, 0); |
There was a problem hiding this comment.
Reader.Create(ArcBuffer, ...) is a new public API, but the committed API baseline file (src/api/Orleans.Serialization/Orleans.Serialization.cs) does not currently list this overload. Update the generated API surface file to include this new method.
| /// Initializes a new instance of the <see cref="ArcBufferReaderInput"/> type. | ||
| /// </remarks> | ||
| /// <param name="slice">The underlying buffer.</param> | ||
| public struct ArcBufferReaderInput(in ArcBuffer slice) |
There was a problem hiding this comment.
ArcBufferReaderInput is a new public type, but it is not present in the committed API baseline (src/api/Orleans.Serialization/Orleans.Serialization.cs). Update the generated API surface file so this type is captured for API review.
This issue also appears on line 121 of the same file.
| public struct ArcBufferReaderInput(in ArcBuffer slice) | |
| internal struct ArcBufferReaderInput(in ArcBuffer slice) |
| public T Deserialize<T>(ArcBuffer source) | ||
| { | ||
| using var session = _sessionPool.GetSession(); | ||
| var reader = Reader.Create(source, session); | ||
| var codec = session.CodecProvider.GetCodec<T>(); | ||
| var field = reader.ReadFieldHeader(); | ||
| return codec.ReadValue(ref reader, field); | ||
| } |
There was a problem hiding this comment.
This new public Deserialize<T>(ArcBuffer source) overload changes the Orleans.Serialization public API surface, but src/api/Orleans.Serialization/Orleans.Serialization.cs does not currently include it. Update the generated API surface file so the baseline matches the new API.
This issue also appears on line 883 of the same file.
| /// <summary> | ||
| /// Creates a writer which writes to the specified destination. | ||
| /// </summary> | ||
| /// <param name="destination">The destination.</param> | ||
| /// <param name="session">The session.</param> | ||
| /// <returns>A new <see cref="Writer{TBufferWriter}"/>.</returns> | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static Writer<ArcBufferWriterWrapper> Create(ArcBufferWriter destination, SerializerSession session) => new(new(destination), session); | ||
|
|
There was a problem hiding this comment.
Writer.Create(ArcBufferWriter, ...) adds new public API surface in a packable src/ project, but src/api/Orleans.Serialization/Orleans.Serialization.cs does not currently include this overload. Update the generated API surface file to reflect the new API.
Summary
ArcBufferinput support for serialization readers, including multi-page reads, position/length tracking, skip, and fork/resume behavior.SerializerandSerializer<T>overloads for deserializing directly fromArcBuffer.Writer.Create(ArcBufferWriter, ...)via a wrapper which does not dispose the underlyingArcBufferWriter.ArcBufferWriterserialization tests for multi-page payloads and serializer round-trips.Microsoft Reviewers: Open in CodeFlow