jamulussoftware / jamulussoftware/jamulus
Evaluate further multithreading improvements
- Dominant language
- C
- Stars
- 1.1k
- Forks
- 248
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 9
Description
**Has this feature been discussed and generally agreed?**
No.
**Describe the solution you'd like**
I think there are multiple places where there is a chance of further improving the multithreading behavior. These are things to investigate and benchmark, so there's no plan to modify this yet.
- [ ] The multithreading code currently [hardcodes the number of threads to the number of visible processor cores](https://github.com/jamulussoftware/jamulus/blob/7d7b337355dce06c7a3b4df5f04ec1084980f695/src/server.cpp#L417-L436). While this might be good for maximum throughput, it might be bad for predictability. In other words: There might be other tasks running on the same machine, e.g. kernel threads which require low latency (network I/O), other Jamulus instances or even other non-Jamulus server processes.
- [ ] We should check if this default makes sense or if $NUM_CORES-1 would be better. Previous discussion: https://github.com/jamulussoftware/jamulus/pull/960#pullrequestreview-585007016 https://github.com/jamulussoftware/jamulus/pull/960#issuecomment-839836327 (If we are unsure whether touching the default is a good idea, we should leave it as-is to avoid causing problems for users)
- [ ] We should look into making this configurable or document how to do it outside of Jamulus (cpuset?). At the very least we should log (on startup) how many threads will be used. Previous discussion: https://github.com/jamulussoftware/jamulus/pull/960#issuecomment-782758759 https://github.com/jamulussoftware/jamulus/pull/960#discussion_r632814416
- [ ] The amount of work which is put into the thread pool is pre-calculated both for [decoding](https://github.com/jamulussoftware/jamulus/blob/7d7b337355dce06c7a3b4df5f04ec1084980f695/src/server.cpp#L802-L823) and [encoding](https://github.com/jamulussoftware/jamulus/blob/7d7b337355dce06c7a3b4df5f04ec1084980f695/src/server.cpp#L878-L896). In other words, on a 5 core machine with 21 clients, 5 threads will be used and blocks of work are generated as 5, 5, 5, 5, 1 (AFAIU).
- [ ] This calculation is sub-optimal. Having one job which does way less than the others doesn't seem logical (of course, depending on client count and number of threads no perfectly balanced distribution is possible). For 21 clients and 5 threads, 5, 4, 4, 4, 4 might be better though.
- [ ] This upfront calculation assumes that Jamulus has full insight into core performance behavior, while in practice it might not have this insight. Cores can be slower due to individual frequency scaling or due to parallel workload (network IO, system software). We don't have insight into the OS scheduler either. So unless the overhead of `pThreadPool->enqueue()` is huge (it requires a lock, so it may be relevant), I'd like to try using smaller blocks of work or dropping the upfront block planning altogether.
- [ ] Find out why a Jamulus server under high load stops responding to protocol messages. Is network I/O the next bottleneck? https://github.com/jamulussoftware/jamulus/blob/master/src/server.cpp#L1476-L1511 looks interesting.
cc'ing @menzels who last touched these parts and might have had good reasons for not going down these paths. :)
Also cc'ing @atsampson, @kraney and @softins who gave valuable input in #960.
Any insights to how benchmarking/profiling was previously done would also be helpful. I'll mainly focus on measuring decoding/mixing duration and jitter using this: https://github.com/jamulussoftware/jamulus/discussions/2440
Contributor guide
Research direction
Start in src/server.cpp at the thread-count setup around lines 417-436, the decoding and encoding work distribution around lines 802-823 and 878-896, and protocol handling around lines 1476-1511. Use the benchmarking approach discussed in Discussion 2440 to measure decoding/mixing duration and jitter; done means reporting the findings and identifying an agreed improvement or documenting why the current behavior should remain.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, performance
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100