Skip to content
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

[WIP] Chunked array for ByteBuffer #7984

Closed
wants to merge 8 commits into from

Conversation

TIHan
Copy link
Contributor

@TIHan TIHan commented Dec 16, 2019

Still a work in progress for this issue: #7982

This one is a bit tricky. Effectively it's writing data to a memory stream that is constantly getting resized and re-allocated when the buffer size limit is reached. Therefore, we need to solve this by having a memory stream backed by chunked byte arrays. There is nothing built into the core libraries that solve this for us; we need to build it ourselves.

Right now I'm using Span for part of it as it helped me reduce some cognitive load. We can take it out if it's an issue. I know I'm having some trouble with the System.Memory package and the compiler.

  • Decide whether or not to use Span (Could use ArraySegment instead)
  • Remove use of ToArray on the ChunkedArray in places we know have the ability to go passed the LOH. ByteMemory Fixed LOH byte array allocations in the IDE from metadata #7971 may need to get merged first before we figure this out.
  • Cleanup - figure out what's public and private for members on ChunkedArray and ChunkedArrayBuilder

Possible Alternatives:

Current impl is based on ideas from System.Reflection.Metadata.BlobBuilder.

@TIHan
Copy link
Contributor Author

TIHan commented Dec 18, 2019

Closing as I think this is too complicated. Will only go back to it if there is no other choice.

@TIHan TIHan closed this Dec 18, 2019
@AlgorithmsAreCool
Copy link

This concept kinda overlaps with ReadOnlySequence. Perhaps merging those concepts and building a SequenceWriter could save some code on the consumption side of things?

There are a few reference implementations floating around for such a thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants