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

Chunked seekable stream reader #566

Merged
merged 15 commits into from
Aug 23, 2023
Merged

Chunked seekable stream reader #566

merged 15 commits into from
Aug 23, 2023

Conversation

halgari
Copy link
Collaborator

@halgari halgari commented Aug 22, 2023

resolves #565

This implements a system for lazy loading of data via chunked streams, main classes implemented in this PR:

  • ChunkedStream - takes a IChunkedStreamSource for loading chunks with a size and count specified by the source. ChunkedStream is a seekable, read-only stream that supports both async and sync read methods. Chunks are stored in a LRU cache that defaults to a size of 16. This should allow for random access to be rather fast, as long as the current working set of data fits within the chunks that fit in the LRU cache
  • IChunkedStreamSource - a interface for defining a chunked stream, has both sync and async GetChunk methods so that each type of threading model remains performant
  • LightweightLRUCache - a very lightweight LRU cache that does not support threading in any manner (we don't need it here), but because of that can be a struct embedded in ChunkedStream.
@halgari halgari requested a review from a team August 22, 2023 03:24
Copy link
Member

@Sewer56 Sewer56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All's good aside from the small notes.

{
if (Find(key) is var index && index != -1)
{
value = _values[index];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can use DangerousGetReferenceAt to eliminate bounds checks.

Copy link
Member

@Sewer56 Sewer56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for merging in current state.

Do consider eliminating some additional bounds checks with DangerousGetReferenceAt where you feel applicable; as it would shave some instructions here and there. That said, all's good now.

@halgari halgari merged commit b68b605 into main Aug 23, 2023
2 of 4 checks passed
@halgari halgari deleted the 565-chunked-loader branch August 23, 2023 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants