339 lines
30 KiB
Markdown
339 lines
30 KiB
Markdown
# ADR 0003: AcBinary streaming receive — AsyncPipeReaderInput unified primitive and transport-agnostic helpers
|
||
|
||
## Status
|
||
|
||
**Accepted (2026-05-03), partially executed** — Steps 1–3 + Step 6 delivered; Steps 4 & 5 dropped during execution.
|
||
|
||
### Execution log
|
||
|
||
| Step | Topic | Original scope | Outcome |
|
||
|------|-------|----------------|---------|
|
||
| 1 | BIN | `AsyncPipeReaderInput.cs` (new sealed class) | ✅ Delivered (`ACCORE-BIN-T-D6H4`, Closed 2026-05-02) |
|
||
| 2 | BIN | `AsyncPipeReaderInputExtensions.DrainFromAsync` | ✅ Delivered, but moved to test-only assembly during Step 1 follow-up (`ACCORE-BIN-T-M2K1`, Closed 2026-05-02) — framework stays consumer-implements-transport rather than exposing a public drain helper. |
|
||
| 3 | BIN | `AcBinarySerializerPipeParallelTests.cs` rewrite — real parallel pipeline test | ✅ Delivered (`ACCORE-BIN-T-V7C9`, Closed 2026-05-02) |
|
||
| 4 | BIN | `AcBinarySerializerNamedPipeExtensions.cs` (NamedPipe helpers) | **❌ Dropped.** Framework decision: stay transport-agnostic, expose only generic `PipeWriter` / `PipeReader` primitives. Tests own `NamedPipeServerStream` / `NamedPipeClientStream` lifecycles directly. See `BINARY_ASYNCPIPE_ISSUES.md#accore-bin-i-t6v2` for the doctrine. |
|
||
| 5 | BIN | `AcBinarySerializerFileStreamExtensions.cs` (FileStream helpers) | **❌ Dropped.** Same rationale as Step 4. Consumers wrap `FileStream` with `PipeWriter.Create` / `PipeReader.Create` themselves. |
|
||
| 6 | SBP | `AcBinaryHubProtocol.cs` migration to `AsyncPipeReaderInput`; `SegmentBufferReader.cs` + `SegmentBufferReaderInput.cs` deleted | ✅ Delivered (`ACCORE-SBP-T-G7T2`, Closed 2026-05-03). Both legacy types removed from disk; protocol now fully on `AsyncPipeReaderInput` (multiMessage:false — protocol parses `[201]/[202]` framing externally, AsyncPipe is a passive byte buffer here). |
|
||
|
||
The body of this ADR below describes the **as-designed** architecture (Steps 1–6). The dropped Steps 4 & 5 do not invalidate the unified-primitive consolidation that motivated the ADR — the receive-side primitive and the SignalR migration both delivered cleanly.
|
||
|
||
### Original status entry (historical)
|
||
|
||
Proposed (2026-04-27)
|
||
|
||
## Context
|
||
|
||
The `AcBinaryHubProtocol` (`AyCode.Services/SignalRs/AcBinaryHubProtocol.cs`) AsyncSegment chunked streaming receive-side currently uses two coupled types in `AyCode.Core/Serializers/Binaries/`:
|
||
|
||
- **`SegmentBufferReader`** (sealed class) — thread-safe single-producer/single-consumer byte buffer with sliding-window semantics (write/read positions reset to 0 when consumer catches up). Backed by `ArrayPool<byte>`. Replaces an earlier `System.IO.Pipelines.Pipe`-based implementation.
|
||
- **`SegmentBufferReaderInput`** (struct) — `IBinaryInputBase` adapter that reads from a `SegmentBufferReader`. Holds only a class-reference; state lives in the buffer class.
|
||
|
||
This receive-side architecture has three structural limitations that block the framework's NuGet positioning and ergonomic standalone use:
|
||
|
||
### 1. Type identity coupled to a SignalR-mode name
|
||
|
||
The "Segment" prefix in both type names refers to AsyncSegment (the SignalR `BinaryProtocolMode`). For a NuGet consumer using `AcBinarySerializer` standalone (NamedPipe IPC, file-stream deserialization), the type names suggest "this is a SignalR thing — won't apply to me", even though the underlying primitive (sliding-window buffer with push/pull feed sources) is fully transport-agnostic.
|
||
|
||
### 2. Send-/receive-side asymmetry without payoff
|
||
|
||
The send side has a clean, single-class primitive: **`AsyncPipeWriterOutput`** (perf-critical hot path, polished). The receive side has a 2-class adapter pair (`SegmentBufferReader` + `SegmentBufferReaderInput`) where the struct is just a thin proxy over the class. The struct/class split was originally motivated by JIT specialization for `DeserializeSequence<TInput>` — but profiling shows the per-call indirection cost is on the order of 1 µs / MB, which is dominated by serialization work itself.
|
||
|
||
### 3. No first-class transport helpers
|
||
|
||
NamedPipe IPC and large-file streaming are common deserialization scenarios for a binary protocol library. Today, consumers must wire up their own `Pipe` + `Task.Run` + manual lifecycle. A self-contained NuGet package should ship turnkey helpers — `SerializeToNamedPipeAsync` / `DeserializeFromNamedPipeAsync` / `SerializeToFileStreamAsync` / `DeserializeFromFileStreamAsync` — sharing the same chunked AsyncSegment wire format.
|
||
|
||
### Wire format (recap)
|
||
|
||
The AsyncSegment chunked wire format is the streaming-deserialization format — a transport-agnostic invariant:
|
||
|
||
```
|
||
[INT32 LE length: 4 byte][message body]
|
||
message body:
|
||
[200] CHUNK_START
|
||
[201][UINT16 size][data] ← repeats
|
||
[201][UINT16 size][data]
|
||
...
|
||
[202] CHUNK_END
|
||
```
|
||
|
||
This format is unchanged by this ADR. The receive-side architecture is the subject; the wire format is the invariant.
|
||
|
||
### Streaming doctrine: peak memory bounded by chunk size
|
||
|
||
The current `SegmentBufferReader.Write` resets `_writePos` and `_readPos` to 0 whenever `_readPos == _writePos > 0` (consumer caught up). This is the central streaming invariant: peak buffer memory is bounded by **two-chunks-worth-of-bytes** (typically ~8 KB), regardless of total message size. A 1-GB file and a 4-KB SignalR message both run with the same ~8-KB peak buffer. This must be preserved across the redesign.
|
||
|
||
### Cross-project scope
|
||
|
||
The change touches both projects in this repo:
|
||
|
||
- **AyCode.Core** (Layer 0, framework): `AsyncPipeReaderInput`, `DrainFromAsync` extension, NamedPipe + FileStream helpers, deletion of `SegmentBufferReader` + `SegmentBufferReaderInput`.
|
||
- **AyCode.Services** (Layer 1): `AcBinaryHubProtocol.TryParseChunkData` migration to delegate to the new primitive; `AsyncChunkState` field type change.
|
||
|
||
ADR placed at the repo-root `docs/adr/` per the multi-project routing rule in [`.github/skills/adr-author/SKILL.md`](../../.github/skills/adr-author/SKILL.md) Step 1 ("cross-cutting decision → highest common ancestor"). The receive-side primitive lives in `AyCode.Core` but is consumed by transport helpers in the same project AND by `AcBinaryHubProtocol` in `AyCode.Services`. The project-scoped sibling [`AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md`](../../AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md) (decorator feature-stack umbrella) depends on this primitive at the AsyncSegment per-chunk decoration boundary; the cross-reference is one-way (Services-ADR depends on Core-ADR).
|
||
|
||
The number `0002` in this folder is intentionally skipped — reserved by [`0001-user-bearer-token-flow.md`](0001-user-bearer-token-flow.md)'s Follow-ups + Related sections for the forthcoming refresh-token flow ADR; preserving that reservation avoids amending an already-Accepted ADR's immutable body.
|
||
|
||
## Decision
|
||
|
||
Consolidate the receive-side architecture around a single transport-agnostic primitive — **`AsyncPipeReaderInput`** — and ship NamedPipe + FileStream helpers using the same chunked AsyncSegment wire format as the SignalR protocol.
|
||
|
||
### 1. `AsyncPipeReaderInput` — single sealed class replaces SegmentBufferReader + SegmentBufferReaderInput
|
||
|
||
A new `sealed class` in `AyCode.Core/Serializers/Binaries/`:
|
||
|
||
```csharp
|
||
public sealed class AsyncPipeReaderInput : IBinaryInputBase, IDisposable
|
||
{
|
||
// Buffer state (sliding window — _readPos/_writePos reset to 0 on consumer catch-up)
|
||
private byte[] _buffer;
|
||
private int _writePos;
|
||
private int _readPos;
|
||
private bool _completed;
|
||
|
||
// Synchronization
|
||
private readonly ManualResetEventSlim _dataAvailable;
|
||
|
||
// Grow tracking (rare path — old buffers held until Dispose to avoid use-after-free
|
||
// while the consumer thread holds a local reference)
|
||
private byte[][]? _oldBuffers;
|
||
private int _oldBufferCount;
|
||
|
||
private readonly ILogger? _logger;
|
||
|
||
// Producer API (push) — used by SignalR's TryParseChunkData and DrainFromAsync alike
|
||
public void Feed(ReadOnlySpan<byte> data);
|
||
public void Complete();
|
||
|
||
// IBinaryInputBase
|
||
public void Initialize(out byte[] buffer, out int position, out int bufferLength);
|
||
public bool TryAdvanceSegment(ref byte[] buffer, ref int position, ref int bufferLength, int needed);
|
||
public void Release();
|
||
|
||
public void Dispose();
|
||
}
|
||
```
|
||
|
||
**Naming convention rationale**: `AsyncPipeReaderInput` is symmetric with the existing `AsyncPipeWriterOutput`. The `Async` **prefix** follows .NET BCL precedent for async-oriented types (`AsyncEnumerable`, `IAsyncDisposable`, `AsyncLocal<T>`, `AsyncCallback`, `IAsyncResult`); the BCL convention puts `Async` as **suffix** on methods (returning `Task` / `ValueTask`) but as **prefix** on types. The `Async` prefix here also differentiates from the BCL `System.IO.Pipelines.PipeReader` type at first glance.
|
||
|
||
`sealed` permits JIT inlining + dynamic PGO devirtualization at `IBinaryInputBase` interface call sites in shared generic code.
|
||
|
||
**Class vs struct rationale**: the struct/class split in today's design exists to keep `DeserializeSequence<TInput>` JIT-specialized over a small struct (the `SegmentBufferReaderInput` proxy). Empirically, the inner deserialization hot path reads from local stack variables (`buffer`, `position`, `bufferLength`) — not through `input.X` — so the struct specialization win is concentrated at the rare-path `TryAdvanceSegment` call (~1 call per chunk worth of bytes). The class-based design pays one extra vtable dispatch on that rare call (~5 ns) in exchange for removing one indirection through the inner buffer reference (~1 ns saved); net delta is sub-microsecond per MB. A pure-struct alternative (with internal buffer state) was rejected — see Alternatives.
|
||
|
||
### 2. Feed-API for push-mode (SignalR pattern)
|
||
|
||
`Feed(ReadOnlySpan<byte> data)` is the producer-thread API for transports that **push** chunk data inline (the SignalR receive loop). The protocol calls `Feed` from `TryParseChunkData` whenever a `[201]` CHUNK_DATA frame's payload arrives. Internal logic:
|
||
|
||
1. If consumer has caught up (`_readPos == _writePos > 0`), reset both to 0 (sliding-window cycling).
|
||
2. If `_writePos + data.Length > _buffer.Length`, grow (last-resort path — under typical chunk-aligned write patterns this never fires).
|
||
3. Copy bytes to the buffer, advance `_writePos` (volatile write), signal `_dataAvailable`.
|
||
|
||
Synchronization model preserved verbatim from today's `SegmentBufferReader`: `_writePos` is volatile-written by producer / volatile-read by consumer; `_readPos` is volatile-written by consumer / volatile-read by producer; reset-to-0 happens only when both equal (consumer not actively reading).
|
||
|
||
### 3. `DrainFromAsync` extension for pull-mode (NamedPipe / FileStream pattern)
|
||
|
||
Transports backed by `System.IO.Pipelines.PipeReader` (NamedPipe and FileStream-via-PipeReader) **pull** data through `PipeReader.ReadAsync()`. An extension method handles this:
|
||
|
||
```csharp
|
||
public static class AsyncPipeReaderInputExtensions
|
||
{
|
||
public static async Task DrainFromAsync(
|
||
this AsyncPipeReaderInput input,
|
||
PipeReader reader,
|
||
CancellationToken cancellationToken = default)
|
||
{
|
||
while (true)
|
||
{
|
||
var result = await reader.ReadAsync(cancellationToken).ConfigureAwait(false);
|
||
foreach (var segment in result.Buffer)
|
||
input.Feed(segment.Span);
|
||
reader.AdvanceTo(result.Buffer.End);
|
||
if (result.IsCompleted) break;
|
||
}
|
||
input.Complete();
|
||
}
|
||
}
|
||
```
|
||
|
||
The extension lives in a separate file (`AsyncPipeReaderInputExtensions.cs`). This keeps the core class free of `System.IO.Pipelines` import surface in its primary contract and makes the optional pull-mode visible at use-sites.
|
||
|
||
### 4. Initial buffer capacity — `BufferWriterChunkSize × 2` heuristic
|
||
|
||
Constructor parameter `initialCapacity` recommended value: `options.BufferWriterChunkSize × 2`.
|
||
|
||
| Context | Chunk size | Initial capacity |
|
||
|---------|-----------|------------------|
|
||
| SignalR (Kestrel slab-aligned) | 4 KB | 8 KB |
|
||
| Standalone (default) | 64 KB | 128 KB |
|
||
|
||
Two-chunks-worth of headroom lets the producer write the next chunk while the consumer is still reading the previous one — without growth. Reset-to-0 cycling reuses the same buffer for the message's lifetime regardless of total payload size.
|
||
|
||
**Explicitly NOT file-size-aware**: an earlier draft considered pre-allocating to file size for `DeserializeFromFileStreamAsync`. Rejected — would defeat the streaming purpose (peak memory ∝ file size, no zerocopy/zeroalloc). The fixed ~8-KB peak (or ~128-KB at the standalone default) is the streaming-doctrine invariant — see Alternatives.
|
||
|
||
### 5. Reset-to-0 sliding-window cycling preserved
|
||
|
||
The peak-memory-bounded streaming invariant from today's `SegmentBufferReader.Write` is preserved verbatim: when `_readPos > 0 && _readPos == _writePos`, both fields reset to 0. Consumer's local `position` re-reads from `_readPos` inside `TryAdvanceSegment` after wake-from-Wait. This is the architectural cornerstone — without it, peak memory grows linearly with message size and the transport-agnostic FileStream helper becomes useless on multi-GB files.
|
||
|
||
### 6. `AsyncPipeWriterOutput` unchanged
|
||
|
||
The send side stays as-is. It's perf-critical and well-tested. This ADR addresses only the receive side. Renaming `AsyncPipeWriterOutput` for full naming uniformity was considered and rejected (breaking change for early NuGet consumers — see Alternatives).
|
||
|
||
### 7. NamedPipe helpers — full lifecycle, one-shot
|
||
|
||
```csharp
|
||
public static class AcBinarySerializerNamedPipeExtensions
|
||
{
|
||
public static async Task SerializeToNamedPipeAsync<T>(
|
||
this AcBinarySerializerOptions options,
|
||
T value,
|
||
string pipeName,
|
||
CancellationToken cancellationToken = default);
|
||
|
||
public static async Task<T?> DeserializeFromNamedPipeAsync<T>(
|
||
this AcBinarySerializerOptions options,
|
||
string pipeName,
|
||
CancellationToken cancellationToken = default);
|
||
}
|
||
```
|
||
|
||
Send-side flow: `NamedPipeServerStream` → `PipeWriter.Create(stream)` → `AsyncPipeWriterOutput` → AcBinary chunked write → flush → close.
|
||
|
||
Receive-side flow: `NamedPipeClientStream` → `PipeReader.Create(stream)` → `AsyncPipeReaderInput.DrainFromAsync(pipeReader)` (background task) → `AcBinaryDeserializer.DeserializeAsync<T>(input)` → result.
|
||
|
||
Cross-platform: `NamedPipeServerStream` / `NamedPipeClientStream` work on Windows + Linux (Unix domain sockets under the hood). WASM throws `PlatformNotSupportedException` per BCL contract.
|
||
|
||
### 8. FileStream helpers — streaming, fixed peak memory
|
||
|
||
```csharp
|
||
public static class AcBinarySerializerFileStreamExtensions
|
||
{
|
||
public static async Task SerializeToFileStreamAsync<T>(
|
||
this AcBinarySerializerOptions options,
|
||
T value,
|
||
string filePath,
|
||
CancellationToken cancellationToken = default);
|
||
|
||
public static async Task<T?> DeserializeFromFileStreamAsync<T>(
|
||
this AcBinarySerializerOptions options,
|
||
string filePath,
|
||
CancellationToken cancellationToken = default);
|
||
}
|
||
```
|
||
|
||
Send-side flow: `FileStream.Create(path)` → `PipeWriter.Create(fileStream)` → `AsyncPipeWriterOutput` → AcBinary chunked write → flush → close.
|
||
|
||
Receive-side flow: `FileStream.OpenRead(path)` → `PipeReader.Create(fileStream)` → same `DrainFromAsync` pattern as NamedPipe.
|
||
|
||
A 1-GB file deserializes with ~8-KB peak buffer (per the streaming-doctrine invariant — Decision #5). Random-access file reads are not used; the file is a one-pass forward stream from the deserializer's perspective.
|
||
|
||
### 9. Unified AsyncSegment chunked wire format across all transports
|
||
|
||
NamedPipe, FileStream, and SignalR all use the same chunked wire format (`[INT32 length][200 CHUNK_START]+[201][UINT16 size][data]+[202 CHUNK_END]`). This is the cornerstone of the unification — one `AsyncPipeReaderInput` implementation handles all three because the wire bytes are identical.
|
||
|
||
Implication: a file written by `SerializeToFileStreamAsync` can be drained by a NamedPipe consumer (with appropriate transport adapter) and produce the same deserialized object. The wire format is the contract; transports are interchangeable.
|
||
|
||
### 10. `AcBinaryHubProtocol.TryParseChunkData` delegates to `AsyncPipeReaderInput.Feed`
|
||
|
||
The existing chunked path in `AcBinaryHubProtocol.TryParseChunkData` (currently uses `state.Buffer.Write(span)` on a `SegmentBufferReader`) migrates to `state.Input.Feed(span)` on an `AsyncPipeReaderInput`. Behavioral equivalence — only the type changes.
|
||
|
||
State storage in `AsyncChunkState`:
|
||
|
||
```csharp
|
||
// Before:
|
||
public SegmentBufferReader Buffer = null!;
|
||
|
||
// After:
|
||
public AsyncPipeReaderInput Input = null!;
|
||
```
|
||
|
||
The cutover preserves the existing protocol mode dispatch, lazy `Task.Run` deser-task start (after first chunk), and `CHUNK_END` lifecycle (`Complete()` + `Dispose()` + `_chunkStates.Remove`). Wire format is unchanged.
|
||
|
||
### 11. UINT16 65535-byte chunk-size invariant — transport-agnostic
|
||
|
||
The `[201][UINT16 size][data]` chunk frame caps wire-chunks at 65535 bytes. This is a **transport-agnostic** invariant — applies to NamedPipe and FileStream chunks, not just SignalR. Consumers configuring `options.BufferWriterChunkSize > 65535` will fail at write-time validation (the existing chunking logic enforces this via the wire format).
|
||
|
||
This ADR codifies the limit as a wire-protocol invariant, not a SignalR-/Kestrel-derived constraint. (See [`AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md`](../../AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md) "UINT16 size prefix design intent" for the original rationale; this ADR generalizes the scope.)
|
||
|
||
### 12. Standalone NuGet positioning — `AcBinarySerializer` self-contained
|
||
|
||
The `AcBinarySerializer` NuGet package (`AyCode.Core` assembly) ships with:
|
||
|
||
- `AsyncPipeReaderInput` — transport-agnostic streaming primitive
|
||
- `DrainFromAsync` extension — `System.IO.Pipelines` consumer
|
||
- `SerializeToNamedPipeAsync` / `DeserializeFromNamedPipeAsync` — NamedPipe IPC
|
||
- `SerializeToFileStreamAsync` / `DeserializeFromFileStreamAsync` — file streaming
|
||
|
||
No transitive `Microsoft.AspNetCore.SignalR` dependency. The SignalR integration (`AcBinaryHubProtocol`) lives in the separate `AyCode.Services` assembly and references `AyCode.Core` for the streaming primitive — not the other way around.
|
||
|
||
## Consequences
|
||
|
||
### Positive
|
||
|
||
- **Single primitive across transports**: SignalR / NamedPipe / FileStream use the same `AsyncPipeReaderInput` — one mental model, one set of bug fixes propagates everywhere.
|
||
- **NuGet-clean self-contained package**: `AcBinarySerializer` ships turnkey transport helpers without forcing consumers to wire `Pipe` + `Task.Run` + lifecycle by hand.
|
||
- **Streaming-doctrine preserved**: ~8-KB peak buffer regardless of message / file size — same as today.
|
||
- **Symmetric send/receive naming**: `AsyncPipeWriterOutput` ↔ `AsyncPipeReaderInput` reads as a pair in the API; the `Async` prefix follows .NET BCL convention for type-level naming.
|
||
- **Type identity decoupled from SignalR**: "Segment" prefix removed; "AsyncPipeReader" describes the class's actual capability (async pipe-style reader), not a SignalR mode.
|
||
- **Easier to test in isolation**: `AsyncPipeReaderInput` can be unit-tested without SignalR scaffolding (which is what the new "real parallel pipeline test" exercises in Step 3 of the migration plan).
|
||
- **Send-side untouched**: `AsyncPipeWriterOutput` (perf-critical, well-tested) stays as-is — risk concentrated on the receive side only.
|
||
|
||
### Negative
|
||
|
||
- **~1 µs / MB perf delta vs current struct+class architecture**: virtual call dispatch on the `IBinaryInputBase` interface in shared generic code path (vs JIT-specialized struct dispatch today). Negligible at typical message sizes (sub-millisecond cost on a 1-GB stream); accepted for the architectural simplicity gain. Verified in Step 3's parallel pipeline test as part of the migration acceptance criteria.
|
||
- **Allocation parity, not improvement**: same 1 class allocation per deserialize (today: `SegmentBufferReader`; after: `AsyncPipeReaderInput`). No regression, no gain.
|
||
- **Migration touches both `AyCode.Core` and `AyCode.Services`**: `AsyncPipeReaderInput` lands in Core; `AcBinaryHubProtocol` migration lands in Services. Two-assembly rollout sequenced via the 6-step migration plan below.
|
||
- **Brief temporary duplication during transition**: between Step 1 (new class lands) and Step 6 (old types deleted), both `SegmentBufferReader` + `SegmentBufferReaderInput` AND `AsyncPipeReaderInput` exist. Bounded by the migration window; mechanical delete at the end.
|
||
|
||
### Migration plan (6 steps, each commit-reviewable)
|
||
|
||
> **Execution outcome (2026-05-03)**: see the **Execution log** at the top of this ADR for what actually shipped. Steps 1–3 + Step 6 delivered as designed; Steps 4 & 5 (NamedPipe / FileStream helpers) dropped — the framework stays consumer-implements-transport. The table below is preserved as the original migration plan for historical context.
|
||
|
||
| Step | Topic | Files | Review checkpoint |
|
||
|------|-------|-------|-------------------|
|
||
| 1 | BIN | `AsyncPipeReaderInput.cs` (NEW); existing `SegmentBufferReader.cs` + `SegmentBufferReaderInput.cs` unchanged | New class compiles, unit-tested in isolation; SignalR path still on old types |
|
||
| 2 | BIN | `AsyncPipeReaderInputExtensions.cs` (NEW — `DrainFromAsync`) | Extension drains an in-memory `Pipe` end-to-end in a unit test |
|
||
| 3 | BIN | `AcBinarySerializerPipeParallelTests.cs` (REWRITE — replaces today's misleading test with a real parallel pipeline test) | Real ser+deser overlap measured on a thread pair; ~1 µs / MB delta confirmed |
|
||
| 4 | BIN | `AcBinarySerializerNamedPipeExtensions.cs` (NEW) | Cross-platform NamedPipe IPC roundtrip test passes |
|
||
| 5 | BIN | `AcBinarySerializerFileStreamExtensions.cs` (NEW) | Large-file (≥ 100 MB) roundtrip test passes with fixed ~8-KB peak buffer |
|
||
| 6 | SBP | `AcBinaryHubProtocol.cs` migrated to `AsyncPipeReaderInput`; `SegmentBufferReader.cs` + `SegmentBufferReaderInput.cs` DELETED | Existing SignalR tests pass; wire format unchanged; code-search finds 0 references to deleted types |
|
||
|
||
Steps 1–5 are tracked under the `BIN` topic ([`AyCode.Core/docs/BINARY/BINARY_TODO.md`](../../AyCode.Core/docs/BINARY/BINARY_TODO.md)). Step 6 is tracked under the `SBP` topic ([`AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/SIGNALR_BINARY_PROTOCOL_TODO.md`](../../AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/SIGNALR_BINARY_PROTOCOL_TODO.md)).
|
||
|
||
The migration is **abortable** between Step 5 and Step 6: if Step 3's parallel pipeline test or Step 4/5's roundtrip tests reveal an unfixable perf regression, the migration is paused; SignalR stays on the old types; the new helpers ship as-is on `AsyncPipeReaderInput` for NamedPipe / FileStream consumers; old types live on indefinitely. Step 6 (delete old types) is the final irreversible step.
|
||
|
||
### Follow-ups required
|
||
|
||
1. **TODO entries** — 6 freshly-generated `ACCORE-BIN-T-<RAND>` and `ACCORE-SBP-T-<RAND>` IDs for the 6 migration steps, added to `AyCode.Core/docs/BINARY/BINARY_TODO.md` (Steps 1–5) and `AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/SIGNALR_BINARY_PROTOCOL_TODO.md` (Step 6) per the `docs-check` skill Step 5 procedure. Tracked as a separate `mehet` round.
|
||
2. **`AyCode.Core/docs/adr/README.md` index update** — append an `0003` row to the index table (Status: Proposed (2026-04-27)). Tracked as a separate `mehet` round.
|
||
3. **ADR-0001 (Services) cross-reference** — [`AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md`](../../AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md) "Related" section can optionally cross-reference ADR-0003 (the per-chunk decoration constraint in AsyncSegment mode benefits from the unified `AsyncPipeReaderInput` primitive). Tracked as a separate `mehet` round.
|
||
4. **Topic-folder cross-references** — [`AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/README.md`](../../AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/README.md) `## Related ADRs` section gets a row for ADR-0003. [`AyCode.Core/docs/BINARY/README.md`](../../AyCode.Core/docs/BINARY/README.md) gets a cross-ref to ADR-0003 for the streaming primitive. Tracked as a separate `mehet` round.
|
||
5. **Promotion to Accepted** — once all 6 migration steps land, Status changes from `Proposed (2026-04-27)` to `Accepted (YYYY-MM-DD)`. Old types deleted, NuGet package self-containment verified by a no-SignalR-reference build check.
|
||
|
||
## Alternatives considered
|
||
|
||
- **Rename existing types in place, preserve 2-class split** (rejected): keep `SegmentBufferReader` + `SegmentBufferReaderInput` as 2 types, just rename to `AsyncPipeReaderBuffer` + `AsyncPipeReaderInput`. Solves the naming-decoupling goal but doesn't solve the proxy split. The struct-as-thin-proxy-over-class indirection survives; users see two types where one would do.
|
||
|
||
- **Struct merger with internal buffer state** (rejected): `struct AsyncPipeReaderInput` carrying `byte[]`, `_writePos`, `_readPos` directly. JIT struct specialization in `DeserializeSequence<TInput>` preserved. Rejected — struct copy semantics break producer/consumer state sharing: the producer (SignalR receive thread) writes to a struct copy, the consumer (deserialization thread) reads from a different copy. Would require all interface methods to take `ref this` or all call sites to pass the struct by `ref` — sprawling API ergonomics issue, sinks into ref-everywhere boilerplate. The class-based design takes the ~1 µs / MB perf hit in exchange for clean copy-by-reference semantics.
|
||
|
||
- **`PipelineReaderInput` naming (no `Async` prefix)** (rejected): cleaner standalone, no BCL `PipeReader` overlap concern, namespace-neutral. Rejected — breaks symmetry with the existing `AsyncPipeWriterOutput` send-side primitive, where the `Async` prefix is established project convention. The asymmetric naming pair would obscure the send/receive symmetry to API readers.
|
||
|
||
- **Rename `AsyncPipeWriterOutput` to match a non-Async receive-side name** (rejected): full uniformity at the cost of a public-API breaking change. Rejected — `AsyncPipeWriterOutput` is part of the existing public surface; renaming would break all early consumers and predates the NuGet positioning effort. The new receive-side name should adopt the established convention, not invert it.
|
||
|
||
- **`Async` as suffix instead of prefix** (rejected): `PipeReaderInputAsync` / `PipeWriterOutputAsync` matching the .NET method-naming convention (`ReadAsync`, `WriteAsync`). Rejected — the `Async` suffix convention applies to **methods** returning `Task` / `ValueTask`, not to types. .NET BCL types use `Async` as **prefix** (`AsyncEnumerable`, `IAsyncDisposable`, `AsyncLocal<T>`, `AsyncCallback`, `AsyncTaskMethodBuilder`, `IAsyncResult`); no BCL type uses an `Async` suffix. A type-suffix `Async` would read as a method name to .NET developers.
|
||
|
||
- **File-size-aware `initialCapacity` for FileStream helpers** (rejected): `DeserializeFromFileStreamAsync` could `Stat` the file and pre-allocate the buffer to file size, achieving "zero growth, single allocation" for the receive buffer. Rejected — defeats the streaming purpose: peak memory becomes O(file size) instead of O(chunk size). A 1-GB file would allocate 1 GB on the heap, eliminating the streaming win. The `BufferWriterChunkSize × 2` heuristic preserves O(chunk size) regardless of file size; the rare grow-path handles the unusual case where chunk-size is misconfigured.
|
||
|
||
- **Composition: `AsyncPipeReaderInput` wrapping a kept `SegmentBufferReader`** (rejected): preserve `SegmentBufferReader` (sealed class) as the buffer primitive, build `AsyncPipeReaderInput` as a thin wrapper that adds Feed/DrainFromAsync API + `IBinaryInputBase`. Rejected — only marginally simpler than today's struct+class split; doesn't realize the "single primitive" goal; users see two types where one suffices. The pure merger is conceptually cleaner.
|
||
|
||
- **`PipeReader`-inheriting type** (rejected): make `AsyncPipeReaderInput` derive from `System.IO.Pipelines.PipeReader`. Rejected — `PipeReader` is an abstract async-buffer reader contract; implementing it commits to all of `ReadAsync` / `AdvanceTo` / `CancelPendingRead` / `Complete` semantics, only ~30% of which apply to the AcBinary streaming use-case. The `IBinaryInputBase` contract is purpose-built for AcBinary deserialization (`Initialize` / `TryAdvanceSegment` / `Release`); inheriting `PipeReader` would force impedance-mismatched API surface for marginal pattern-match value.
|
||
|
||
- **Place ADR at `AyCode.Services/docs/adr/`** (rejected): an earlier draft placed this ADR in the project-scoped folder as ADR-0002 (sibling of the decorator-feature-stack ADR). Rejected — the change touches both `AyCode.Core` (primary, primitive + helpers) and `AyCode.Services` (protocol delegation). Per the multi-project routing rule, cross-cutting ADRs live at the highest common ancestor — repo-root `docs/adr/`. The project-scoped folder remains for project-internal decisions.
|
||
|
||
## Related
|
||
|
||
- [`AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md`](../../AyCode.Services/docs/adr/0001-acbinary-decorator-feature-stack-design.md) — project-scoped umbrella for `AcBinaryHubProtocol` optional feature stack. ADR-0003's `AsyncPipeReaderInput` is the receive-side base primitive that the decorator ADR's per-chunk decorators (compression / encryption / signing) operate on top of in AsyncSegment mode.
|
||
- Send-side counterpart: `AsyncPipeWriterOutput` in `AyCode.Core/Serializers/Binaries/`. Unchanged by this ADR.
|
||
- Wire format: [`AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/README.md`](../../AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/README.md) documents the AsyncSegment chunked wire format (the invariant this ADR generalizes to NamedPipe + FileStream transports).
|
||
- Topic homes: [`AyCode.Core/docs/BINARY/README.md`](../../AyCode.Core/docs/BINARY/README.md) (Core-side primitive + helpers), [`AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/README.md`](../../AyCode.Services/docs/SIGNALR_BINARY_PROTOCOL/README.md) (Services-side protocol delegation).
|
||
- Number `0002` in this folder skipped: reserved by [`0001-user-bearer-token-flow.md`](0001-user-bearer-token-flow.md) Follow-ups for the forthcoming refresh-token flow ADR.
|
||
- Upcoming TODO entries: 6 in BIN + SBP topics — see Follow-up #1.
|