boost::thread_resource_error improvements

[edolstra]

Motivation

• Make boost::thread_resource_error a non-fatal error, so long as there is at least one eval thread. Not sure if this will actually help, since if we're at the process limit, then presumably any other thread/process creation will fail as well.
• Limit the default number of eval cores to 32.

Context

Summary by CodeRabbit

• Bug Fixes

  • Improved evaluator worker startup: clearer error when no workers can be created and a warning/report when only a subset start successfully.

• Chores

  • Capped default evaluation core allocation (when profiling is disabled) to a practical maximum to avoid excessive core use.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 752a5721-a7e4-46d6-98ba-5d5a6c30a662

📥 Commits

Reviewing files that changed from the base of the PR and between 2b8879d and 5c862d7.

📒 Files selected for processing (1)

• src/libexpr/parallel-eval.cc

🚧 Files skipped from review as they are similar to previous changes (1)

• src/libexpr/parallel-eval.cc

---

📝 Walkthrough

Walkthrough

Adjusted parallel evaluator core selection and hardened worker-thread creation. Executor::getEvalCores now caps the derived default cores to at most 32 when evalProfilerMode is disabled and evalCores == 0. Executor constructor catches boost::thread_resource_error, throwing if zero workers were created or warning and stopping on partial creation.

Changes

Cohort / File(s)

Summary

Executor Thread Management src/libexpr/parallel-eval.cc

Executor::getEvalCores: when evalProfilerMode == disabled and evalCores == 0, derive cores as min(32, Settings::getDefaultCores()) instead of using Settings::getDefaultCores() directly. Executor::Executor: wrap worker thread creation in try/catch for boost::thread_resource_error; if first creation fails (n==0) throw Error("could not create any evaluator worker threads: %s", e.what()); if a later creation fails (n!=0) call warn("could only create %d evaluator worker threads: %s", n, e.what()) and break the creation loop.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I’m a rabbit in the thread-filled glen,
I cap the cores where wild defaults ken,
I catch the errors, tidy the spill,
Warn or fail with composed goodwill,
Hop on—workers start, safe and still 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: handling boost::thread_resource_error as non-fatal and limiting default eval thread count to 32.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch eelcodolstra/nix-378

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🚀 Deployed on https://69e1034395225f0f67cb3d52--determinate-nix.netlify.app

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/libexpr/parallel-eval.cc`:
- Around line 35-44: The loop should base fallback and the "enabled" mode on the
number of workers actually created rather than the requested evalCores: if
evalCores == 1 skip creating threads and leave parallel mode disabled; otherwise
perform the createWorker loop but after the loop compute size_t actual_workers =
state->threads.size() and use that to decide behavior (if actual_workers == 0
throw the original fatal Error, if actual_workers == 1 set executor as
disabled/serial and log a warning that only 1 worker was created, otherwise set
enabled = true); also update the existing log messages to reference
actual_workers instead of the loop index n so the reported count matches
state->threads.size().

🪄 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: b16c9616-d92e-4320-8e56-ed3eb7d8ca3d

📥 Commits

Reviewing files that changed from the base of the PR and between b5c0faf and 2b8879d.

📒 Files selected for processing (1)

• src/libexpr/parallel-eval.cc

[cole-h]

Would it be useful to add a small note explaining that this 32 figure is just because (as the commit message says) scalability sees diminishing returns? In case we ever fix these scalability issues, so we can again unlock much more parallelism...