Size the engine from detected hardware unless manual tuning is on - #88
Merged
Conversation
appsettings.json is the base configuration for every environment, and it pinned the profile measured for 20 cores / 500 parallel workflows: 256/768 runspaces, 768 ThreadPool threads, 600 dispatch workers. The production template rolled the same numbers onto arbitrary hardware via Install-NodePilot.ps1. On a smaller host that over-provisions badly — 768 minimum ThreadPool threads already measured a 28% regression on a 20-core box — while the code defaults underneath were CPU-scaled and would have landed on the documented "8-Core / 20 par. WFs" row exactly. Performance:ManualTuning (default false) now decides. Off, the runspace pool, step cap, ThreadPool floor and dispatch queue are derived from the detected CPU and memory; on, the configured values are used verbatim. The measured profile stays in every template as an inert, immediately activatable preset rather than being deleted. What this produces is a safe, monotonically scaling default with bounded resource risk, not a universal optimum: that additionally depends on workflow count, activity mix, step duration and remote latency, none of which are knowable at boot. Automatic mode therefore targets the light and moderate rows of the documented load table and stops at ceilings taken from measurements (600 steps is the Sperrvermerk optimum; 64 runspaces is the moderate-load row). The measured 768-runspace profile is reachable only through the switch, so automatic sizing can never silently downgrade it. Memory is a single shared household with explicit sub-budgets rather than one budget per knob, which would spend the same bytes several times over; a test asserts the invariant across a CPU-by-RAM grid. Because the automatic ceilings are conservative, memory only binds on genuinely constrained hosts such as cgroup-limited containers — tests pin both that case and its complement. Detection uses ProcessorCount and GC.GetGCMemoryInfo().TotalAvailableMemoryBytes so container limits are honoured, and falls back to CPU-only sizing when the reading is absent or implausible. The plan is resolved once at boot and shared as a singleton. Resolving it live would leave the hot-reloadable ThreadPool tuned for one mode while the boot-fixed runspace pool and dispatch queue ran in the other; the Settings UI shows the desired mode against the active one and asks for a restart instead. Engine:MaxConcurrentExecutions is deliberately excluded. It is a safety cap against trigger loops and sub-workflow cascades, not a throughput lever, so an explicitly configured value must keep applying in both modes — an earlier revision folded it into the plan and the capacity tests wedged waiting for a rejection that never came. The sizing arithmetic is a pure function in Core taking detected hardware as values; the Api owns detection and deployment posture, since Core cannot reference it. A new effective-sizing endpoint reports what is actually in force and which constraint produced each value — EffectiveSource carries only configuration source names and cannot show that the stored numbers are inert.
Comment on lines
+50
to
+55
| catch (Exception) | ||
| { | ||
| // Never let hardware detection stop the host from booting — CPU-only sizing is a | ||
| // perfectly safe fallback. | ||
| return null; | ||
| } |
Comment on lines
+184
to
+189
| foreach (var cores in new[] { 1, 2, 4, 8, 12, 16, 20, 32, 64, 128 }) | ||
| { | ||
| var value = Auto(cores, ramGb: 256).MaxConcurrentSteps.Value; | ||
| value.Should().BeGreaterThanOrEqualTo(previous, "adding cores must never shrink the plan"); | ||
| previous = value; | ||
| } |
Comment on lines
+196
to
+201
| foreach (var ramGb in new[] { 2, 4, 8, 16, 32, 64, 128, 256 }) | ||
| { | ||
| var value = Auto(cores: 32, ramGb).MaxRunspaces.Value; | ||
| value.Should().BeGreaterThanOrEqualTo(previous, "adding memory must never shrink the plan"); | ||
| previous = value; | ||
| } |
This was referenced Jul 31, 2026
The plan was resolved and used but never written anywhere, so the effective sizing was invisible in the field — an operator comparing two hosts had no way to see what was detected, what was chosen, or which constraint bound each value. It is logged as soon as the bootstrap logger exists; the plan itself is resolved earlier because the ThreadPool prewarm needs it before that point.
Test-DeploymentTemplates.ps1 parses them with ConvertFrom-Json under Windows PowerShell 5.1, which rejects the // comments I had added — ASP.NET Core would have tolerated them, the contract check does not. The rationale moves to deploy/README.md, which now also states the constraint so the next edit does not repeat it.
…ance-sizing # Conflicts: # docs/performance-improvements.md
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
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.
Why
src/NodePilot.Api/appsettings.jsonis the base configuration for every environment, and it pinned the profile measured for 20 cores / 500 parallel workflows (256/768 runspaces, 768 ThreadPool threads, 600 dispatch workers).deploy/Install-NodePilot.ps1rolled the same numbers onto arbitrary hardware through the production template. On a smaller host that over-provisions badly — 768 minimum ThreadPool threads already measured a 28% regression on a 20-core box — while the CPU-scaled code defaults underneath would have landed on the documented "8-Core / 20 par. WFs" row exactly.What changed
Performance:ManualTuning(default false) decides:Scope of the promise. This produces a safe, monotonically scaling default with bounded resource risk, not a universal optimum — that additionally depends on workflow count, activity mix, step duration and remote latency, none of which are knowable at boot. Automatic mode therefore targets the light/moderate rows of the documented load table and stops at ceilings taken from measurements (600 steps is the Sperrvermerk optimum; 64 runspaces is the moderate-load row). The measured 768-runspace profile is reachable only through the switch, so automatic sizing can never silently downgrade it.
Memory is one shared household with explicit sub-budgets, not a budget per knob — the latter would spend the same bytes several times over. A test asserts the invariant across a CPU-by-RAM grid. Because the automatic ceilings are conservative, memory only binds on genuinely constrained hosts (cgroup-limited containers); tests pin that case and its complement. Detection honours container limits via
ProcessorCount+GC.GetGCMemoryInfo().TotalAvailableMemoryBytes, with a CPU-only fallback when the reading is absent or implausible.Boot snapshot. The plan is resolved once and shared as a singleton. Resolving it live would leave the hot-reloadable ThreadPool tuned for one mode while the boot-fixed runspace pool and dispatch queue ran in the other. The Settings UI shows desired vs. active mode and asks for a restart.
Engine:MaxConcurrentExecutionsis deliberately excluded — a safety cap against trigger loops and sub-workflow cascades, not a throughput lever, so an explicitly configured value keeps applying in both modes.Sizing arithmetic is a pure function in Core taking hardware as values; the Api owns detection and deployment posture (Core cannot reference it). A new
GET /api/admin/settings/effective-sizingreports what is in force and which constraint produced each value —EffectiveSourcecarries only configuration source names and cannot express that the stored numbers are inert.Verification
Engine 1596/1596 · Api 2171/2171 · frontend 65/65 ·
tscclean. Two defects were caught by the full-suite run and fixed: derivingMinRunspacesfromMaxRunspacesreintroduced eager pre-warm (a measured anti-pattern) and wedged the runner at 421 threads; folding the execution safety caps into the plan madeWorkflowEngineCapacityTestswait forever for a rejection that never came.Docs:
docs/performance-improvements.mdgains the formulas, budget model and the honest note on when memory actually binds — and corrects the "~30 MB per runspace" rule of thumb, which came from the process-spawn path and overstated sizing guidance by a factor of ~20 (measured: 1.2–1.4 MB).