NixOS / hydra

Hydra, the Nix-based continuous build system
http://nixos.org/hydra
GNU General Public License v3.0
1.1k stars 291 forks source link

queue-runner: only re-sort runnables by prio once per dispatch cycle #1301

Closed delroth closed 7 months ago

delroth commented 9 months ago

The previous implementation was O(N²lg(N)) due to sorting the full runnables priority list once per runnable being scheduled. While not confirmed, this is suspected to cause performance issues and bottlenecking with the queue runner when the runnable list gets large enough.

This commit changes the dispatcher to instead only sort runnables per priority once per dispatch cycle. This has the drawback of being less reactive to runnable priority changes: the previous code would react immediately, while this might end up using "old" priorities until the next dispatch cycle. However, dispatch cycles are not supposed to take very long (seconds, not minutes/hours), so this is not expected to have much or any practical impact.

Ideally runnables would be maintained in a sorted data structure instead of the current approach of copying + sorting in the scheduler. This would however be a much more invasive change to implement, and might have to wait until we can confirm where the queue runner bottlenecks actually lie.


I don't really have a Hydra instance to test this with, but @Ma27 reported that his instance seemingly worked fine with this patch applied.

Note also that almost all of the diff is moving code, unfortunately the change in indentation level makes it avoid move detection :)

dasJ commented 7 months ago

We've been using this in our internal Hydra for some time now and I haven't seen any issues with the queue runner.

Mindavi commented 7 months ago

I also tried this out without issues for a while.