fix: read output streams straight from native memory - #4
Closed
msallin wants to merge 1 commit into
Closed
Conversation
OutputStream overrode only the byte-array reads. Because the runtime type is then no longer exactly UnmanagedMemoryStream, its Read(Span) stops taking the direct path and defers to Stream.Read(Span), which rents an array the size of the caller's span from the shared ArrayPool, reads into it, copies it out and returns it to the pool without clearing it. That has two consequences. Every span-shaped or asynchronous read copies each byte twice, and the rendered document is left in a process-wide pool where the next component to rent from it can read the bytes back - the same disclosure PooledBuffer clears its buffer to avoid. Overriding Read(Span) to copy from the pointer removes both. It does not recurse; that would need a call back into base.Read(Span). The byte-array overload now funnels into it, and ReadAsync picks it up for free.
msallin
force-pushed
the
feat/output-stream-span-read
branch
from
September 6, 2026 19:16
d52fdc2 to
c5d0fe9
Compare
Member
Author
|
Reopened upstream against the parent repository: evolvedlight#47 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OutputStreamoverrode only the byte-array reads. Because the runtime type is then no longer exactlyUnmanagedMemoryStream, itsRead(Span)defers toStream.Read(Span), which rents an array fromArrayPool<byte>.Shared, reads into it, copies it out, and returns it to the pool without clearing it.Two consequences:
UnmanagedMemoryStream.PooledBufferclears its buffer specifically to prevent this.Overriding
Read(Span)to copy from the pointer removes both and restores parity with a plainUnmanagedMemoryStream(0.274 ms). The byte-array overload funnels into it, andReadAsync(Memory)picks it up becauseUnmanagedMemoryStream.ReadAsynccalls the virtualRead(Span).The replaced comment claimed that overriding
Read(Span)would recurse. It does not: recursion would require calling back intobase.Read(Span).CopyTois unaffected. It uses an 81920-byte rented buffer like every other stream, not a document-sized one.Tests
SpanReadLeavesNoDocumentBytesInTheSharedArrayPoolprimes the pool, performs a span read, and rents again to assert the document is not there. Confirmed failing against the previous implementation before the fix was applied. Also added: oversized-span and end-of-stream boundaries, chunked reads that do not divide evenly, and theReadAsync(Memory)path.60/60 tests pass. Builds clean on net8, net9 and net10 with zero warnings.