feat(config): replace hardcoded thread defaults with dynamic CPU detection by glottologist · Pull Request #1242 · Irys-xyz/irys

glottologist · 2026-03-25T15:25:07Z

Summary

Before:
cpu_packing_concurrency and parallel_verification_thread_limit were hardcoded to fixed values (2 and 4 respectively), with TODO comments noting they should derive from CPU count. Operators on machines with more or fewer cores than expected got suboptimal defaults. The VdfNodeConfig struct required an explicit [vdf] section in TOML — omitting it caused deserialisation failure.

After:
Both fields default to max(1, available_cpus - 4) at runtime via std::thread::available_parallelism(). The [vdf] section and cpu_packing_concurrency field are now optional in TOML configs, falling back to the dynamic default when omitted.

Changes

Core helpers (`crates/types/src/config/node.rs`)

Add thread_count_with_reserve(available, reserved) — pure function: available.saturating_sub(reserved).max(1)
Add default_thread_count(reserved) — wrapper that reads available_parallelism() and delegates to the pure function
Update LocalPackingConfig::default() to use default_thread_count(4) for cpu_packing_concurrency
Update VdfNodeConfig::default() to use default_thread_count(4) for parallel_verification_thread_limit
Add #[serde(default)] at struct level on VdfNodeConfig so the entire [vdf] section becomes optional
Update testing_with_signer() and testnet() constructors to use the dynamic helper

Config templates

Remove cpu_packing_concurrency = 4 from mainnet_config.toml and testnet_config.toml
Remove [vdf] / parallel_verification_thread_limit = 4 sections from both templates

Docker configs (8 files)

Remove cpu_packing_concurrency and parallel_verification_thread_limit from all docker config files under docker/configs/, docker/agent_cluster/configs/, and docker/tests/data-sync/configs/

Tests

Add 3 proptests for thread_count_with_reserve: always >= 1, correct subtraction, floor at 1
Add regression test for partial [vdf] section deserialisation (only core_pinning present)
Update TOML test strings in config/mod.rs to omit the now-optional fields

Technical Notes

Breaking changes: None. Existing TOML configs with explicit values still deserialise correctly (covered by the legacy regression test). Only the defaults change for configs that omit the fields.
Architecture: The pure thread_count_with_reserve function is separated from the system call wrapper to enable property-based testing without mocking.

Testing

Property-based tests for the pure helper (3 proptests)
Partial [vdf] section deserialisation regression test
Legacy config deserialisation regression test unchanged and passing
Full irys-types test suite (566 tests pass)
Clippy clean across full workspace
cargo xtask check compiles cleanly

Summary by CodeRabbit

Enhancements
- Threading now adapts to available CPUs with dynamic defaults for packing and VDF parallel-verification limits.
Chores
- Cleaned up configuration templates and examples: removed hardcoded CPU packing and VDF thread-limit entries; several example configs reduced explicit CPU packing values (e.g., 4 → 2) and fixed trailing-newline formatting.

coderabbitai · 2026-03-25T15:25:21Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ca89543-c625-4f87-8b3a-5ab55372b485

📥 Commits

Reviewing files that changed from the base of the PR and between 3352792 and f8c67ab.

📒 Files selected for processing (4)

crates/config/templates/mainnet_config.toml
crates/config/templates/testnet_config.toml
crates/types/src/config/mod.rs
crates/types/src/config/node.rs

💤 Files with no reviewable changes (2)

crates/config/templates/testnet_config.toml
crates/config/templates/mainnet_config.toml

📝 Walkthrough

Walkthrough

Replace hardcoded thread/concurrency defaults with dynamic CPU-based calculations; remove explicit CPU packing and VDF thread-limit entries from TOML templates so runtime defaults apply; add helper functions and tests for thread-count behavior and adjust serde defaults for VDF config.

Changes

Config runtime and templates

Layer / File(s)	Summary
Runtime helpers, defaults, and tests `crates/types/src/config/node.rs`	Add `thread_count_with_reserve()` and `default_thread_count()`; use `default_thread_count(4)` for `LocalPackingConfig::default()` and `VdfNodeConfig::default()`; change VDF serde to include `default`; add proptest and TOML deserialization tests.
Inline TOML fixture updates `crates/types/src/config/mod.rs`	Reposition `[vdf]` in the inline TOML fixture and set `parallel_verification_thread_limit = 4` in tests to reflect dynamic-default behavior.
Config templates `crates/config/templates/mainnet_config.toml`, `crates/config/templates/testnet_config.toml`	Remove `packing.local.cpu_packing_concurrency` entries and top-level `[vdf]` sections so runtime defaults are used.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: replacing hardcoded thread defaults with dynamic CPU detection, which is the core objective of this PR.
Linked Issues check	✅ Passed	The PR fully implements all coding requirements from `#1101`: dynamic CPU-based defaults, consistent calculation logic, pure helper function for testing, serde defaults for optional VDF section, and removal of hardcoded values.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing dynamic thread detection: code helpers, config defaults, template/fixture updates, and corresponding tests. No unrelated modifications found.

_{✏️ 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 jason/1101-implement-dynamic-thread-detection-for-vdfnodeconf

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

glottologist · 2026-03-25T15:27:32Z

Implement Dynamic Thread Detection for VdfNodeConfig and LocalPackingConfig — Design Plan

Mode: Feature
Clarity Score: 0.97 (Goal: 1.0, Constraints: 0.95, Scope: 0.95)
Date: 2026-03-25

Problem Statement

VdfNodeConfig::parallel_verification_thread_limit and LocalPackingConfig::cpu_packing_concurrency use hardcoded defaults (4 and 2 respectively). On machines with many cores these values underutilize available parallelism; on machines with few cores they may contend with other work. Both structs carry TODO comments requesting dynamic defaults based on CPU count.

Architecture & Approach

Shared helper function. A single pure function thread_count_with_reserve(available, reserved) computes max(1, available - reserved). A convenience wrapper default_thread_count(reserved) calls std::thread::available_parallelism() and feeds the result into the pure function. Each call site chooses its own reserved value (currently 4 for both VDF and packing).

This mirrors the existing default_actix_workers() pattern at node.rs:441-445, which uses available_parallelism() with a halving formula. That function stays separate because it uses division rather than subtraction — different formula, same principle.

Serde integration. LocalPackingConfig already has #[serde(deny_unknown_fields, default)] at struct level, so missing TOML fields fall through to Default. VdfNodeConfig lacks this — adding #[serde(default)] at struct level allows both fields to be optional in TOML, enabling dynamic defaults when operators omit explicit values.

Config templates updated. Mainnet and testnet TOML templates drop the explicit parallel_verification_thread_limit and cpu_packing_concurrency lines so the dynamic defaults take effect. Operators who want to override can re-add the field — serde deserialization picks up explicit values over defaults.

Test configs adapt. NodeConfig::testing_with_signer() replaces its hardcoded values with the dynamic helper. Docker test configs also drop explicit values. This means tests scale to the CI machine's CPU count, matching production behavior.

Components

Modified

File	Change
`crates/types/src/config/node.rs`	Add `thread_count_with_reserve()` + `default_thread_count()` helpers. Update `VdfNodeConfig` (add `#[serde(default)]`, update `Default` impl). Update `LocalPackingConfig::default()`. Update `testing_with_signer()`.
`crates/config/templates/mainnet_config.toml`	Remove `parallel_verification_thread_limit` and `cpu_packing_concurrency` lines
`crates/config/templates/testnet_config.toml`	Remove `parallel_verification_thread_limit` and `cpu_packing_concurrency` lines
`docker/configs/irys-1.toml`	Remove `parallel_verification_thread_limit` and `cpu_packing_concurrency` lines
`docker/agent_cluster/configs/irys-1.toml`	Remove `parallel_verification_thread_limit` and `cpu_packing_concurrency` lines
`docker/tests/data-sync/configs/irys-1.toml`	Remove `parallel_verification_thread_limit` and `cpu_packing_concurrency` lines

No external dependencies added

std::thread::available_parallelism() is stable since Rust 1.59. No need for num_cpus.

Trade-offs

Dynamic defaults mean different behavior per machine. Acceptable because that is the explicit goal — adapt to available hardware. Operators retain the ability to override via TOML.
Test behavior varies by CI machine CPU count. Acceptable per user decision. The minimum floor of 1 thread prevents failures on small machines.
default_actix_workers() not refactored. It uses a fundamentally different formula (division). Forcing it through the same helper would require a more complex interface for no real benefit.

Out of Scope

GPU packing batch size changes (unrelated to CPU thread detection)
Refactoring default_actix_workers() to use the new helper
Adding CLI flags or environment variable overrides for thread counts
Per-storage-module concurrency tuning

Verification

cargo xtask check compiles cleanly
cargo clippy --workspace --tests --all-targets passes with no warnings
cargo nextest run -p irys-types passes — proptest validates the pure helper function
cargo nextest run full suite passes — no test regressions from dynamic defaults
On a machine with N cores, VdfNodeConfig::default().parallel_verification_thread_limit == max(1, N - 4) and LocalPackingConfig::default().cpu_packing_concurrency == max(1, N - 4) as u16

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docker/agent_cluster/configs/irys-1.toml (1)
59-61: ⚠️ Potential issue | 🟡 Minor

Add explicit cpu_packing_concurrency limits back to the fixture TOMLs—no compose-side CPU caps are present.

The three-node cluster in docker/agent_cluster/docker-compose.yaml runs without per-container CPU limits (no cpus, cpuset, or cpu_quota directives). With cpu_packing_concurrency removed from the configs, each node will default to available_parallelism() - 4 and can oversubscribe the host.

Restore explicit low concurrency values in [packing.local] (e.g., cpu_packing_concurrency = 1 or 2) for the agent cluster and data-sync fixture TOMLs to prevent resource contention during testing on shared hosts. Alternatively, add CPU limits to each service in the compose manifests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker/agent_cluster/configs/irys-1.toml` around lines 59 - 61, The
[packing.local] section removed cpu_packing_concurrency which lets each node
default to available_parallelism()-4 and can oversubscribe hosts; restore an
explicit low limit (e.g., add cpu_packing_concurrency = 1 or 2) inside the
[packing.local] table in this TOML (and the other agent-cluster and data-sync
fixture TOMLs) to cap packing concurrency, or alternatively add per-service CPU
limits in the docker-compose manifests; update the cpu_packing_concurrency
setting for the relevant fixture configs and ensure the same low value is
applied across the three-node cluster to prevent resource contention.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/types/src/config/node.rs`:
- Around line 1013-1014: Take a single dynamic snapshot of
default_thread_count(4) in each NodeConfig builder and reuse it for both packing
and VDF concurrency fields instead of calling default_thread_count(4) twice;
e.g. in NodeConfig::testing_with_signer and NodeConfig::testnet compute let
thread_count = u16::try_from(default_thread_count(4)).unwrap_or(u16::MAX) (or
keep the intermediate usize and convert once) and assign that value to
cpu_packing_concurrency and cpu_vdf_concurrency (and the other duplicated sites
at the referenced locations) to remove duplicate conversion logic and keep the
defaults aligned.

---

Outside diff comments:
In `@docker/agent_cluster/configs/irys-1.toml`:
- Around line 59-61: The [packing.local] section removed cpu_packing_concurrency
which lets each node default to available_parallelism()-4 and can oversubscribe
hosts; restore an explicit low limit (e.g., add cpu_packing_concurrency = 1 or
2) inside the [packing.local] table in this TOML (and the other agent-cluster
and data-sync fixture TOMLs) to cap packing concurrency, or alternatively add
per-service CPU limits in the docker-compose manifests; update the
cpu_packing_concurrency setting for the relevant fixture configs and ensure the
same low value is applied across the three-node cluster to prevent resource
contention.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c0debc11-b2b0-4b32-b9f7-8bfaf16981fa

📥 Commits

Reviewing files that changed from the base of the PR and between 46f6611 and 7947119.

📒 Files selected for processing (12)

crates/config/templates/mainnet_config.toml
crates/config/templates/testnet_config.toml
crates/types/src/config/mod.rs
crates/types/src/config/node.rs
docker/agent_cluster/configs/irys-1.toml
docker/agent_cluster/configs/irys-2.toml
docker/agent_cluster/configs/irys-3.toml
docker/configs/irys-1.toml
docker/configs/irys-2.toml
docker/tests/data-sync/configs/irys-1.toml
docker/tests/data-sync/configs/irys-2.toml
docker/tests/data-sync/configs/irys-3.toml

💤 Files with no reviewable changes (5)

docker/configs/irys-2.toml
docker/configs/irys-1.toml
crates/config/templates/mainnet_config.toml
crates/config/templates/testnet_config.toml
crates/types/src/config/mod.rs

github-actions · 2026-03-25T15:43:07Z

Benchmark results: https://irys-xyz.github.io/irys/dev/bench/1242%2Fmerge/index.html

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 `@docker/tests/data-sync/configs/irys-2.toml`:
- Line 61: Add an inline comment next to the cpu_packing_concurrency = 2 setting
explaining why this Docker test fixture pins CPU packing to 2 (e.g., to keep
deterministic CI behavior, avoid oversubscription on small test runners, or to
match downstream service limits) so future maintainers understand why it
overrides the new dynamic defaults; update the irys-2.toml entry for
cpu_packing_concurrency with a concise one-line rationale referencing
CI/determinism or hardware constraints.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 99819455-8cbd-4dea-8d70-8a87357aa1f5

📥 Commits

Reviewing files that changed from the base of the PR and between 9dfc7f9 and 0c5c0a9.

📒 Files selected for processing (6)

docker/agent_cluster/configs/irys-1.toml
docker/agent_cluster/configs/irys-2.toml
docker/agent_cluster/configs/irys-3.toml
docker/tests/data-sync/configs/irys-1.toml
docker/tests/data-sync/configs/irys-2.toml
docker/tests/data-sync/configs/irys-3.toml

…ction

…nfigs

JesseTheRobot

LGTM

coderabbitai Bot reviewed Mar 25, 2026

View reviewed changes

Comment thread crates/types/src/config/node.rs Outdated

coderabbitai Bot reviewed Mar 25, 2026

View reviewed changes

Comment thread docker/tests/data-sync/configs/irys-2.toml

glottologist requested review from DanMacDonald, JesseTheRobot and roberts-pumpurs March 26, 2026 12:31

glottologist added 6 commits May 15, 2026 09:59

feat(config): replace hardcoded thread defaults with dynamic CPU dete…

96e99cc

…ction

refactor(config): snapshot default_thread_count once per builder

9e7cd07

fix(docker): cap cpu_packing_concurrency at 2 in fixture configs

bf36286

docs(docker): explain cpu_packing_concurrency pin in fixture configs

3f78b43

fix(docker): pin parallel_verification_thread_limit in all fixture co…

71599ce

…nfigs

fix(config): restore fixed thread counts in test constructors

f8c67ab

JesseTheRobot force-pushed the jason/1101-implement-dynamic-thread-detection-for-vdfnodeconf branch from 9919948 to f8c67ab Compare May 15, 2026 10:07

JesseTheRobot approved these changes May 15, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(config): replace hardcoded thread defaults with dynamic CPU detection - #1242

feat(config): replace hardcoded thread defaults with dynamic CPU detection#1242
glottologist wants to merge 6 commits into
masterfrom
jason/1101-implement-dynamic-thread-detection-for-vdfnodeconf

glottologist commented Mar 25, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 25, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

glottologist commented Mar 25, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Mar 25, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

JesseTheRobot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

glottologist commented Mar 25, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Core helpers (crates/types/src/config/node.rs)

Config templates

Docker configs (8 files)

Tests

Technical Notes

Testing

Related

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

glottologist commented Mar 25, 2026

Implement Dynamic Thread Detection for VdfNodeConfig and LocalPackingConfig — Design Plan

Problem Statement

Architecture & Approach

Components

Modified

No external dependencies added

Trade-offs

Out of Scope

Verification

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JesseTheRobot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

glottologist commented Mar 25, 2026 •

edited by coderabbitai Bot

Loading

Core helpers (`crates/types/src/config/node.rs`)

coderabbitai Bot commented Mar 25, 2026 •

edited

Loading

github-actions Bot commented Mar 25, 2026 •

edited

Loading