Add an overload for BrowseAsync that accepts a delegate for handling each directory#935
Open
jpdillingham wants to merge 5 commits into
Open
Add an overload for BrowseAsync that accepts a delegate for handling each directory#935jpdillingham wants to merge 5 commits into
jpdillingham wants to merge 5 commits into
Conversation
|
Is there any particular reason you want to use an Action callback for every item, rather than using AsyncEnumerables or System.IO.Pipelines? Those seem to be easier to compose. An idea for how to keep the responsibilities separate: the TCP reader handles the outer framing and spawns a task to consume the Zlib-compressed content (transmitted over a pipe), that task decompresses it and sends the compressed content over a second pipe, and the third task uses a small state machine to gradually consume chunks from the pipe and emit the inner directory entries. |
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
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.
Currently
BrowseAsyncreturns the entire browse response as a C# object, which Soulseek.NET must build in memory from the decompressed payload sent from the other client. Consumers must then take this C# object and read/manipulate it, possibly store it after serializing it to JSON.For extremely large shares, the current approach puts a lot of memory pressure on Soulseek.NET and the containing application, requiring a copy of both the compressed and decompressed payloads in memory concurrently, at least.
This PR introduces a new overload of
BrowseAsyncthat accepts anAction<Directory, bool>that's called by Soulseek.NET for each directory contained within a browse response, after decompressing the incoming response on the fly. The bool is used to indicate whether the directory is locked, as there is no property associated with files or directories that carries this information today.This PR is WIP that's predicated on the successful implementation of on-the-fly decompression and directory building that matches or beats the performance of the existing solution. The successful technique will require staging chunks of data and searching for start and end markers for directories to re-assemble them, and I'm not confident that 1) this is even possible with the current ZLib implementation or 2) that this can be done in a memory efficient way (will require borrowed arrays from a buffer) and 3) that it won't be CPU intensive (doubtful, but who knows!)
If implemented successfully, this will allow implementing applications to serialize directory objects as they are received, avoiding potential OOM issues that are possible with the current approach.
Closes #889