Implement concurrent recursion when appending items to the playlist #59
Conversation
This file contains 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
this means that the flag can be changed from any point in the appending process
This feature has the potential for massive performance improvements when recursively opening large directories while using addons with asynchronous IO.
the mpv event queue has a limited size, if we don't limit the number of concurrent parses then we risk overfloing the queue which will cause some directories to be dropped
this is already handled by the concurrent_parse function, there is no need to check this again
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.
When recursing through a directory to append its contents to the playlist run
each parse operation concurrently.
This feature has the potential for massive speed improvements when recursively
opening large directories while using addons with asynchronous IO.
This feature can bring addon performance more in-line with the native
parser (which is usually faster for local filesystems).
Network file systems in particular will benefit from this significantly.
The only downsides compared to the default loadlist that I am currently aware of
are higher CPU load (mpv and file-browser are less responsive), and less-predictable append speeds.
The overall time it takes to append the whole directory tree will certainly be less, but it may take longer
to load the first or any subsequent items.
If the number of concurrent recursions is too high then there is the risk of the mpv event
queue overflowing. The mpv event queue is seemingly limited to 1000 items, but only async
mpv actions like command_native_async should use that, events like mp.add_timeout should
be handled entirely on the Lua side with a table, which has a significantly larger maximum size.
Therefore, as long as the max_concurrency value is kept well below 1000 (to allow for addons
creating more than one event each) we should not be at risk.
If this feature is enabled when not using asynchronous IO the behaviour
should be roughly the same as the default, but performance may degrade slightly.
As this is still an experimental feature it will be disabled by default.