From 527d87613eb23ee0f3b9404f0a10fbb824ab4ee8 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 14:39:39 +0100 Subject: [PATCH] Without the composer's share a step has no thread limit; with it, the share over the module's own capacity (ADR 0553 round 2) Review round 2: change A must not slow builds before any slice exists, so a module given no share sets no GOMAXPROCS. The slice's quota bounds the total; threads only size one step, from the node's share over the module's own capacity (shanks: 24 x 50% / 3 = 4). The share is the node-engine's own config. --- go/buildcpu/buildcpu.go | 59 ++++++++++++++--------------------- go/buildcpu/buildcpu_test.go | 30 +++++++++--------- go/buildstep/capacity_test.go | 13 +++----- go/buildstep/cpu.go | 7 ++--- go/catalogue/seats_test.go | 12 +++---- 5 files changed, 51 insertions(+), 70 deletions(-) diff --git a/go/buildcpu/buildcpu.go b/go/buildcpu/buildcpu.go index 06b9dec..dcd2f36 100644 --- a/go/buildcpu/buildcpu.go +++ b/go/buildcpu/buildcpu.go @@ -8,13 +8,14 @@ // gives its runtime process declaration.Yields, and the node-engine places every yielding process of the node in // one slice (declaration.YieldSlice) whose CPU weight is idle, with Nice=19 for every process it starts. // Interactive and service work always comes first; builds take the cycles nothing else wants. -// - **One cap per node, shared.** The node's share of its processors is one value, the node-tools runtime's setting +// - **One cap per node, shared.** The node's share of its processors is one value, the node-engine's own config // `build-cpu-share` (ShareSetting), set per node through the node's settings layer. The node-engine writes it as the // slice's CPUQuota: one bound the kernel holds for all the node's build modules together, never one per module. -// - **Threads.** Inside that bound a step starts no more threads than its part of it (Threads): the node's share -// divided by every build slot of the node, across all its build modules (the composer hands both as words, -// ShareWord and SlotsWord). GOMAXPROCS sets every go command's -p, the compiler's concurrency, a test binary's -// processors and -parallel. +// - **Threads.** The quota bounds the total; threads only size each step. A step gets the share divided by its own +// module's capacity (Threads), from the share the composer hands as a word (ShareWord): GOMAXPROCS sets every go +// command's -p, the compiler's concurrency, a test binary's processors and -parallel. A module given no word sets +// no thread limit at all: without the slice there is nothing to size a step to, and a limit then would only slow +// builds. package buildcpu import ( @@ -24,7 +25,7 @@ import ( "strings" ) -// ShareSetting is the node-tools runtime's setting of the node's processor share for its builds, in percent of the +// ShareSetting is the node-engine's config value of the node's processor share for its builds, in percent of the // machine's processors, 1 to 100: one value per node, set through the node's settings layer (novox/hq ADR 0553 §3). const ShareSetting = "build-cpu-share" @@ -32,16 +33,8 @@ const ShareSetting = "build-cpu-share" // itself whatever builds. const DefaultPercent = 50 -// BuildSeats is how many seats yield (catalogue.Seat.Yields): one build module per language. A step that is told no -// node-wide slot count assumes every one of them runs its full capacity beside it. -const BuildSeats = 3 - -// The words the composer gives every yielding module (novox/hq ADR 0553 §4): the node's share, and every build slot -// of the node across all its yielding modules. -const ( - ShareWord = "MESH_BUILD_CPU_SHARE" - SlotsWord = "MESH_BUILD_SLOTS" -) +// ShareWord is the word the composer gives every yielding module (novox/hq ADR 0553 §4): the node's share, in percent. +const ShareWord = "MESH_BUILD_CPU_SHARE" // Quota is a CPUQuota for a share of a machine with cores processors: systemd counts a quota in percent of one // processor, so half of 24 is 1200%. At least 100%, one processor, so a share on a small machine never stops a step. @@ -53,8 +46,8 @@ func Quota(percent, cores int) int { return q } -// Threads is how many threads one step may run at once: its part of the share when slots steps may run on the node -// at once, at least one. Rounded down, so the steps together stay inside the quota. +// Threads is how many threads one step may run at once: its part of the share when its own module runs slots steps at +// once, at least one. The slice's quota bounds the steps of all the node's build modules together; this only sizes one. func Threads(percent, cores, slots int) int { if slots < 1 { slots = 1 @@ -71,44 +64,38 @@ func Threads(percent, cores, slots int) int { // Limits is what one step is held to on this node, as a status tool says it. type Limits struct { - // Percent is the node's share in percent of its processors; From says where it was read: the composer's word, - // or the default. - Percent int `json:"share-percent"` + // Percent is the node's share in percent of its processors, and From where it was read: ShareWord, or "none" when + // the composer gave no share and a step runs without a thread limit. + Percent int `json:"share-percent,omitempty"` From string `json:"share-from"` // Cores is the machine's processor count. Cores int `json:"cores"` - // Slots is every build slot of the node, across all its build modules; SlotsFrom where it was read. - Slots int `json:"node-slots"` - SlotsFrom string `json:"slots-from"` - // Threads is what each step is given (Threads). + // Slots is this module's own capacity on the node: steps at once. + Slots int `json:"slots"` + // Threads is what each step is given (Threads); 0 is no limit. Threads int `json:"threads-per-step"` } -// FromWords is the limits of a step from the words the composer gave this process (ShareWord, SlotsWord). Where a word -// is absent or not a number: the default share, and own × BuildSeats slots. A module told nothing assumes every build -// module of the node runs its full capacity beside it, so the steps together never take more than the default share. +// FromWords is the limits of a step from the share the composer gave this process (ShareWord) and the module's own +// capacity. Without a usable share there is no limit: Threads is 0. func FromWords(getenv func(string) string, own int) Limits { if own < 1 { own = 1 } - l := Limits{Percent: DefaultPercent, From: "default", Cores: runtime.NumCPU(), Slots: own * BuildSeats, - SlotsFrom: "own capacity × " + strconv.Itoa(BuildSeats) + " build seats"} + l := Limits{From: "none", Cores: runtime.NumCPU(), Slots: own} if n, err := strconv.Atoi(strings.TrimSpace(getenv(ShareWord))); err == nil && n >= 1 && n <= 100 { l.Percent, l.From = n, ShareWord + l.Threads = Threads(n, l.Cores, own) } - if n, err := strconv.Atoi(strings.TrimSpace(getenv(SlotsWord))); err == nil && n >= 1 { - l.Slots, l.SlotsFrom = n, SlotsWord - } - l.Threads = Threads(l.Percent, l.Cores, l.Slots) return l } // Env is the environment a step's commands are given so they start no more threads than they may: GOMAXPROCS for every // Go program (the compiler, a test binary, esbuild, buildah), UV_THREADPOOL_SIZE for Node's worker pool. Returned as -// KEY=value words to append to a command's environment. +// KEY=value words to append to a command's environment; nothing for 0 threads, no limit. func Env(threads int) []string { if threads < 1 { - threads = 1 + return nil } t := strconv.Itoa(threads) return []string{"GOMAXPROCS=" + t, "UV_THREADPOOL_SIZE=" + t} diff --git a/go/buildcpu/buildcpu_test.go b/go/buildcpu/buildcpu_test.go index cdf7b91..6740687 100644 --- a/go/buildcpu/buildcpu_test.go +++ b/go/buildcpu/buildcpu_test.go @@ -8,18 +8,17 @@ import ( "testing" ) -// The node's share as a quota, the threads one step is given across every build slot of the node, and the words the -// composer hands, in one table (ADR 0553, issue 1806). +// The node's share as a quota, the threads one step is given for its own module's capacity, and the word the composer +// hands, in one table (ADR 0553, issue 1806). func TestShareQuotaAndThreads(t *testing.T) { for _, c := range []struct { name string percent, cores, slots int quota, threads int }{ - {"shanks: half of 24 across 9 slots", 50, 24, 9, 1200, 1}, - {"shanks: half of 24 across 4 slots", 50, 24, 4, 1200, 3}, - {"ace: three quarters of 24 across 6 slots", 75, 24, 6, 1800, 3}, - {"g14: half of 16 across 1 slot", 50, 16, 1, 800, 8}, + {"shanks: half of 24, three slots", 50, 24, 3, 1200, 4}, + {"ace: three quarters of 24, two slots", 75, 24, 2, 1800, 9}, + {"g14: half of 16, one slot", 50, 16, 1, 800, 8}, {"one processor at least", 10, 2, 4, 100, 1}, {"no slots counts as one", 100, 4, 0, 400, 4}, {"a share out of range is the default", 0, 8, 1, 0, 4}, @@ -33,17 +32,18 @@ func TestShareQuotaAndThreads(t *testing.T) { t.Errorf("%s: Threads = %d, want %d", c.name, th, c.threads) } } - words := map[string]string{ShareWord: "75", SlotsWord: "6"} - l := FromWords(func(k string) string { return words[k] }, 2) - if l.Percent != 75 || l.From != ShareWord || l.Slots != 6 || l.SlotsFrom != SlotsWord || l.Threads != Threads(75, runtime.NumCPU(), 6) { - t.Errorf("from the words: %+v", l) + l := FromWords(func(k string) string { return map[string]string{ShareWord: "75"}[k] }, 2) + if l.Percent != 75 || l.From != ShareWord || l.Slots != 2 || l.Threads != Threads(75, runtime.NumCPU(), 2) { + t.Errorf("from the word: %+v", l) } - // Told nothing (or nonsense), a module assumes every build seat beside it at its own capacity, at the default share. - words = map[string]string{ShareWord: "250", SlotsWord: "many"} - if l := FromWords(func(k string) string { return words[k] }, 3); l.Percent != DefaultPercent || l.Slots != 3*BuildSeats || l.From != "default" { - t.Errorf("without usable words: %+v", l) + // Told nothing usable, a step has no thread limit: nothing in its environment. + for _, word := range []string{"", "250", "lots"} { + l := FromWords(func(string) string { return word }, 3) + if l.Threads != 0 || l.From != "none" || Env(l.Threads) != nil { + t.Errorf("share %q: %+v, env %v", word, l, Env(l.Threads)) + } } - if got := Env(0); !reflect.DeepEqual(got, []string{"GOMAXPROCS=1", "UV_THREADPOOL_SIZE=1"}) { + if got := Env(3); !reflect.DeepEqual(got, []string{"GOMAXPROCS=3", "UV_THREADPOOL_SIZE=3"}) { t.Errorf("Env = %q", got) } } diff --git a/go/buildstep/capacity_test.go b/go/buildstep/capacity_test.go index 632cdfc..a9d1a60 100644 --- a/go/buildstep/capacity_test.go +++ b/go/buildstep/capacity_test.go @@ -37,17 +37,14 @@ func TestNodeCapacity(t *testing.T) { } } -// ADR 0553 §4: a step's limits come from the composer's words; without them, every build seat at the module's own -// capacity at the default share. +// ADR 0553 §4: a step's threads are the composer's share over the module's own capacity; without the word, none. func TestNodeLimits(t *testing.T) { t.Setenv("MESH_BUILD_CPU_SHARE", "") - t.Setenv("MESH_BUILD_SLOTS", "") - if l := NodeLimits(3); l.Percent != 50 || l.Slots != 9 { - t.Errorf("without words: %+v", l) + if l := NodeLimits(3); l.Threads != 0 || l.Slots != 3 { + t.Errorf("without the word: %+v", l) } t.Setenv("MESH_BUILD_CPU_SHARE", "75") - t.Setenv("MESH_BUILD_SLOTS", "5") - if l := NodeLimits(3); l.Percent != 75 || l.Slots != 5 || l.Threads < 1 { - t.Errorf("from the words: %+v", l) + if l := NodeLimits(3); l.Percent != 75 || l.Slots != 3 || l.Threads < 1 { + t.Errorf("from the word: %+v", l) } } diff --git a/go/buildstep/cpu.go b/go/buildstep/cpu.go index 6147202..39b5e3e 100644 --- a/go/buildstep/cpu.go +++ b/go/buildstep/cpu.go @@ -6,10 +6,9 @@ import ( "git.novox.be/novox/mesh-sdk/go/buildcpu" ) -// NodeLimits is what one step of a build module is held to (novox/hq ADR 0553 §4): the node's processor share and every -// build slot of the node, from the words the composer gives the module (buildcpu.ShareWord, buildcpu.SlotsWord), and the -// machine's processors. A module told neither assumes every build seat beside it at its own capacity, own, so its steps -// stay within the default share with the others'. +// NodeLimits is what one step of a build module is held to (novox/hq ADR 0553 §4): the node's processor share from the +// word the composer gives the module (buildcpu.ShareWord), divided by the module's own capacity, own, over the machine's +// processors. Without the word, no thread limit: Threads is 0. func NodeLimits(own int) buildcpu.Limits { return buildcpu.FromWords(os.Getenv, own) } diff --git a/go/catalogue/seats_test.go b/go/catalogue/seats_test.go index 4e65fca..62043e1 100644 --- a/go/catalogue/seats_test.go +++ b/go/catalogue/seats_test.go @@ -4,8 +4,6 @@ import ( "regexp" "strings" "testing" - - "git.novox.be/novox/mesh-sdk/go/buildcpu" ) // Defends novox/hq ADR 0110: a seat is a module assignment from a closed set. @@ -337,14 +335,14 @@ func TestNodeBuildAgentIsRetiredForTheBuildSeats(t *testing.T) { t.Errorf("%s serves %v", name, verbs) } } - // The build seats are the seats that yield, as many as a step told nothing assumes beside it (ADR 0553 §4). - yielding := 0 + // The build seats are the seats that yield, and no other (ADR 0553 §1). + var yielding []string for _, s := range Seats() { if s.Yields { - yielding++ + yielding = append(yielding, s.Name) } } - if yielding != buildcpu.BuildSeats { - t.Errorf("%d seats yield, and buildcpu.BuildSeats says %d", yielding, buildcpu.BuildSeats) + if strings.Join(yielding, ",") != "node-build-go,node-build-node,node-build-image" { + t.Errorf("the seats that yield are %v", yielding) } }