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

perf: chunk and process playback segments concurrently #3663

Closed

Conversation

boenshao
Copy link

The /list API can be a bit slow when there are lots of recordings that need to be processed.

The concatenation procedure can be split up and executed concurrently (and even parallelly).

I hard-coded the splits into 4, as most modern platforms have at least 4 cores.

But it's possible to introduce a more sophisticated way to determine the preferred number, either by the number of files or the total size of the files. We can gather the statistics in os.Walk.

Yet I believe the overhead of goroutine is negligible in most cases, so a static strategy is probably alright.

With this patch, on my AlderLake laptop, the response time is about 3x faster when parsing 2X GBs of recordings.

@boenshao boenshao force-pushed the perf/optmize-playback-list branch 3 times, most recently from ed0caa0 to 2a8b2f1 Compare August 16, 2024 15:32
@boenshao boenshao force-pushed the perf/optmize-playback-list branch from 2a8b2f1 to f05f946 Compare August 27, 2024 03:04
@boenshao boenshao force-pushed the perf/optmize-playback-list branch from f05f946 to e95cd74 Compare September 30, 2024 09:17
@boenshao boenshao force-pushed the perf/optmize-playback-list branch from e95cd74 to d39ea20 Compare October 24, 2024 03:04
@aler9
Copy link
Member

aler9 commented Jan 2, 2025

Hello, the main bottleneck of the /list endpoint was caused by the fact that the duration of each segment was computed by reading every part of the segment and summing their duration. This is fixed in #4096.

The idea behind this PR is that performing parallel reads on a disk improves performance. I've gathered some documentation on the topic, in particular this article is quite interesting:
https://pkolaczk.github.io/disk-parallelism/

Basically, parallel readings improve performance with SSDs but decrease performance with HDDs when readings are sequential. And HDDs are still popular for 24/7 video recordings.

Therefore: this patch is interesting but i'd like to take it into consideration after evaluating whether #4096 is enough for providing decent performance in most cases, which will take some time (it applies only to new segments and not to existing ones).

aler9 added a commit that referenced this pull request Jan 3, 2025
Segments are now parsed in parallel.
@aler9
Copy link
Member

aler9 commented Jan 3, 2025

This PR has an additional issue: it prevents more than 4 segments from being grouped together. The /list endpoint is supposed to provide timespans as long as possible, in order for them to be represented in a UI, not the segment list, which can be quite large.

This is replaced by #4102 which parses segments in parallel but performs concatenation in series.

@aler9 aler9 closed this Jan 3, 2025
aler9 added a commit that referenced this pull request Jan 3, 2025
Segments are now parsed in parallel.
Copy link
Contributor

github-actions bot commented Jan 3, 2025

This issue is mentioned in release v1.11.0 🚀
Check out the entire changelog by clicking here

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