nix search: Reduce evaluator worker starvation - #564
Merged
Conversation
Off-CPU profiling showed that ~2 of 12 worker threads were idle on average during the parallel phase of 'nix search', because work items were only spawned after fully enumerating each attrset: while one thread enumerated legacyPackages.x86_64-linux (~120k attributes), and later each large package subset (pythonPackages, perlPackages, ...), the other workers had nothing new to pick up. Two changes: * Spawn work items incrementally (every 256 attributes) during enumeration instead of in one batch at the end, so idle workers can start on the first attributes while enumeration continues. * Executor::spawn(): Generate queue keys with a thread-local mt19937_64 instead of calling std::random_device per work item, which costs hundreds of cycles per call (RDRAND / /dev/urandom). The key only needs to spread same-priority items around the queue, not be cryptographically random. At 12 eval cores (with GC disabled via GC_INITIAL_HEAP_SIZE=40G), this reduces elapsed time from ~4.6s to ~4.3s, user CPU from ~28s to ~26s, and kernel time from ~5s to ~3.9s for 'nix search nixpkgs --no-eval-cache fizzbuzz'. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe changes update executor work-item key generation to use a thread-local PRNG and make search evaluation submit work incrementally when batches reach 256 items. ChangesParallel evaluation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
cole-h
approved these changes
Jul 14, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/libexpr/parallel-eval.cc`:
- Around line 137-145: Update the thread-local
std::uniform_int_distribution<uint64_t> dist in the key-generation code to use
(1ULL << 48) - 1 as its inclusive upper bound, keeping the random suffix
confined to bits 0–47 and preventing overlap with the priority field shifted by
48.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bad6783-defa-42f0-b5f1-bcaee8c0b88b
📒 Files selected for processing (2)
src/libexpr/parallel-eval.ccsrc/nix/search.cc
Comment on lines
+137
to
+145
| /* Note: this uses a cheap PRNG rather than std::random_device, | ||
| since the latter costs hundreds of cycles per call (RDRAND or | ||
| /dev/urandom), which adds up when spawning many work items. The | ||
| key only needs to spread items of the same priority around the | ||
| queue, not be cryptographically random. */ | ||
| [[gnu::tls_model("initial-exec")]] static thread_local std::mt19937_64 rng{std::random_device{}()}; | ||
| [[gnu::tls_model("initial-exec")]] static thread_local std::uniform_int_distribution<uint64_t> dist( | ||
| 0, 1ULL << 48); | ||
| auto key = (uint64_t(item.second) << 48) | dist(rd); | ||
| auto key = (uint64_t(item.second) << 48) | dist(rng); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
! rg -n '0,\s*1ULL << 48\)' src/libexpr/parallel-eval.ccRepository: DeterminateSystems/nix-src
Length of output: 200
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Show the relevant section with line numbers.
sed -n '120,160p' src/libexpr/parallel-eval.cc
printf '\n---\n'
# Find the type/usage of the queued item and key comparisons.
rg -n 'item\.second|key =|priority|uniform_int_distribution|mt19937_64|queue' src/libexpr/parallel-eval.ccRepository: DeterminateSystems/nix-src
Length of output: 2531
🌐 Web query:
std::uniform_int_distribution inclusive upper bound cppreference
💡 Result:
In C++, the std::uniform_int_distribution template produces random integers that are uniformly distributed on the closed interval [a, b] [1][2]. This means that both the minimum value (a) and the maximum value (b) are inclusive [1][3]. When constructing the distribution, the second parameter b defaults to the maximum representable value of the integer type (std::numeric_limits::max), which is also included in the potential output range [4][2].
Citations:
- 1: https://en.cppreference.com/cpp/numeric/random/uniform_int_distribution
- 2: https://eel.is/c++draft/rand.dist.uni.int
- 3: https://learn.microsoft.com/en-us/cpp/standard-library/uniform-int-distribution-class?view=msvc-170
- 4: https://en.cppreference.com/cpp/numeric/random/uniform_int_distribution/uniform_int_distribution
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate the relevant type definitions and queue ordering.
rg -n 'WorkItems|Priority|queue|std::priority_queue|std::map|emplace\(key|item\.second' src/libexpr/parallel-eval.cc
printf '\n---\n'
# Show the top of the file where types and state are declared.
sed -n '1,120p' src/libexpr/parallel-eval.ccRepository: DeterminateSystems/nix-src
Length of output: 4664
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Find the declarations used by Executor::spawn and the queue type.
rg -n 'using WorkItems|struct Item|queue|priority|enum class|enum ' src/libexpr/parallel-eval.hh src/libexpr/parallel-eval.cc
printf '\n--- parallel-eval.hh ---\n'
sed -n '1,220p' src/libexpr/parallel-eval.hhRepository: DeterminateSystems/nix-src
Length of output: 1103
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate the parallel-eval headers and the WorkItems / queue declarations.
fd -a 'parallel-eval.hh' src
printf '\n---\n'
fd -a 'parallel-eval.cc' src
printf '\n---\n'
rg -n 'using WorkItems|struct Item|priority|queue|std::map|std::priority_queue|enum class' src/libexpr src -g '!**/build/**' || trueRepository: DeterminateSystems/nix-src
Length of output: 50382
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Find the actual location of the parallel-eval header and inspect declarations.
fd -a 'parallel-eval.hh' .
printf '\n---\n'
fd -a 'parallel-eval.cc' .
printf '\n---\n'
rg -n 'using WorkItems|struct Item|priority|queue|std::map|std::priority_queue|enum class' src lib -g '!**/build/**' || trueRepository: DeterminateSystems/nix-src
Length of output: 50382
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate the Executor/State declaration and the queue container type.
rg -n 'struct State|class Executor|queue' src -g '*parallel*' -g '*eval*' -g '!**/build/**'
printf '\n---\n'
sed -n '1,220p' $(rg -l 'struct State|class Executor' src -g '*parallel*' -g '*eval*' | head -n 1)Repository: DeterminateSystems/nix-src
Length of output: 7519
Keep the random suffix within bits 0–47. std::uniform_int_distribution is inclusive, so 1ULL << 48 can be returned and set bit 48, overlapping the priority field. Use (1ULL << 48) - 1 instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/libexpr/parallel-eval.cc` around lines 137 - 145, Update the thread-local
std::uniform_int_distribution<uint64_t> dist in the key-generation code to use
(1ULL << 48) - 1 as its inclusive upper bound, keeping the random suffix
confined to bits 0–47 and preventing overlap with the priority field shifted by
48.
1 task
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
2 participants
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.
Motivation
Small improvements from #534: spawn work items faster so that idle workers can start earlier, and use a faster PRNG.
Context
Summary by CodeRabbit