feat(compiler): memory-aware concurrency limit

[RafaelGranza]

Auto-size Sierra compilation concurrency from available memory

What

Today the number of concurrent Sierra→CASM compilations defaults to GOMAXPROCS, which ignores memory. A burst of large compilations can exhaust RAM.
This PR derives the limit from available memory and CPU count.

Formula (all values in MB):

limit = min( (availableMemory - nodeMemoryReserve) / maxMemoryPerCompilation , CPUs )

The limit is always at least 1: a memory-tight node compiles one at a time rather than refusing to start.

Behavior change (heads-up for operators)

The default concurrency is no longer always GOMAXPROCS. On a many-core but memory-modest host it drops: e.g. 16 CPU / 16 GB with defaults goes from 16 to (16-4)/4 = 3. Intended, but nodes with heavy P2P or getCompiledCasm load will see less compile parallelism. Raise --max-concurrent-compilations (used as-is) or --node-memory-reserve to tune.

Flags

Flag	Default	Meaning
--max-concurrent-compilations	unset	unset (flag not provided) = derive from memory + CPUs. A provided value is used as-is (an explicit 0 disables compilations).
--max-compilation-queue	unset	unset = twice the resulting concurrency. A provided value is used as-is.
--node-memory-reserve	4096	RAM (MB) kept for the rest of the node, out of the compilation budget. New.
--max-compilation-memory	4096	RAM (MB) one compilation may use. Existing; it is the divisor above.

Compilation-sizing flags are parsed as unsigned integers by the CLI, so a non-numeric or negative value is rejected at flag-parse time. Whether a flag was provided (across CLI, env, or YAML) is what selects derive-vs-explicit, so an explicit 0 is a valid, distinct value.

Why these libraries

Two needs: read total RAM, and read the container memory limit. Choice:

Need	Library	Why
Host/VM RAM	pbnjay/memory	One function, pure Go, no cgo, all OSes.
Container limit	automemlimit	Purpose-built for GOMEMLIMIT, pure Go, reads cgroup v1/v2.

Both were already indirect dependencies; this only promotes them to direct.
automemlimit's only dependency is pbnjay/memory, so the pair adds no new transitive dependency.

gopsutil was considered and dropped: it does not read the cgroup limit (same blind spot as pbnjay), and it would be an extra dependency on top of pbnjay (which automemlimit pulls anyway).

Container awareness

A cgroup is the Linux kernel feature that caps how much memory a process group may use; it is how Docker and Kubernetes enforce memory limits. A process can sit on a 64 GiB host but be capped to 4 GiB by its cgroup, and the kernel kills it (OOM) if it goes over.

Reading total system memory (sysinfo / /proc/meminfo) returns the host RAM and ignores cgroups, so inside a memory-limited container it over-reports.

AvailableMemoryMB() instead returns min(host RAM, cgroup limit), so it respects docker run --memory=.. and Kubernetes limits.

Environment	host-only reading	this PR
Bare metal / no limit	host RAM	host RAM
Container with memory limit	host RAM (wrong)	cgroup limit
macOS / Windows (no cgroup)	host RAM	host RAM

Margin default

--node-memory-reserve defaults to 4 GiB, a conservative reserve for the rest of the node. No deeper analysis; it is configurable. Operators with a large --db-cache-size or many VMs can raise it.

Testing

Verified by running the node and reading the startup log
(Sierra compilation limits).

Environment	Config	availableMemory	concurrent compilations	Result
macOS native	default	16384 (host)	10 (CPUs)	boots
macOS native	--max-compilation-memory 4096	16384	3	boots
macOS native	--node-memory-reserve 100000	16384	1 (floored)	boots
Docker --memory=8g	default	8192 (cgroup)	1	boots
Docker --memory=6g	default	6144 (cgroup)	1 (floored)	boots

The Docker rows confirm the value follows the cgroup limit, not the 16 GiB host.

When the budget fits no compilation the limit floors to 1, so the node boots and compiles one at a time instead of refusing to start.

[github-actions]

Claude finished @RafaelGranza's task in 3m 47s —— View job

---

PR Review: memory-aware concurrency limit

• Read changed files
• Review starknet/compiler/concurrency.go (core logic)
• Review node/node.go wiring + fail-fast
• Review cmd/juno/juno.go flags
• Review tests
• Post review

---

Reviewed for correctness, the cgroup/container divergence, the fail-fast path, and the flag wiring. This is a clean, well-tested PR — no blockers or correctness bugs. One inline nit and a few notes below.

Blocker

None.

Important

None.

Nit

• node/node.go:319 — misleading log fields when concurrency is overridden (inline). Also answers your question on the log: keep it, just consider gating the memory fields (or adding a source field) so they don't imply they drove a value that was actually set by flag.

Things I checked that are correct

• compilerInUse set is complete. cfg.HTTP || cfg.Websocket || cfg.P2P || cfg.Sequencer covers every code path that actually invokes throttledCompiler — the RPC handler (getCompiledCasm), the P2P service (p2p.New(..., throttledCompiler)), and the sequencer. Plain feeder sync downloads pre-compiled CASM and never compiles, so a default sync-only node correctly skips the fail-fast and falls through to max(..., 1). Good.
• No underflow. ConcurrencyLimit guards availableMemory <= nodeMemoryReserve before the uint64 subtraction, so the unsigned subtract can't wrap. The maxMemoryPerCompilation == 0 short-circuit also avoids a divide-by-zero.
• cgroup logic is sound. cgroupLimit > 0 && cgroupLimit < hostMemory correctly treats the "unlimited" cgroup (max value ≥ host) as "use host RAM", and FromCgroup() returns an error on non-Linux so it falls back to host memory. Units are consistent (bytes compared in bytes, then /megabyte).
• Tests are deterministic, not time-based: NodeMemoryReserve: AvailableMemoryMB() + 4096 guarantees nothing fits regardless of host RAM, and the two new node tests cover both the reject-on-zero-budget and ignore-when-no-compiler branches. The table tests hit the boundary cases (==reserve, <reserve+one, cap-by-parallel, unbounded).
• Dependency claim holds — automemlimit's only dep is pbnjay/memory; both move from indirect → direct with no new transitive deps.

One heads-up (not a bug)

On a many-core but memory-modest host (e.g. 16 CPU / 16 GB, defaults), derived concurrency drops from the old GOMAXPROCS (16) to (16-4)/4 = 3. That's the intended behavior, but it's a meaningful default change for existing deployments — worth a line in the release notes / changelog so operators on P2P or heavy getCompiledCasm workloads aren't surprised by reduced compile parallelism.

[codecov]

Codecov Report

❌ Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.32%. Comparing base (b611c5c) to head (63a269b).

Files with missing lines	Patch %	Lines
starknet/compiler/concurrency.go	86.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3764      +/-   ##
==========================================
+ Coverage   75.24%   75.32%   +0.07%     
==========================================
  Files         439      440       +1     
  Lines       39397    39442      +45     
==========================================
+ Hits        29645    29709      +64     
+ Misses       7671     7655      -16     
+ Partials     2081     2078       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Claude finished @RafaelGranza's task in 2m 58s —— View job

---

PR Review: memory-aware concurrency limit

• Read changed files
• Review starknet/compiler/concurrency.go (core logic)
• Review node/node.go wiring + fail-fast + log
• Review cmd/juno/juno.go flags + tests
• Build + vet affected packages
• Post review

This PR is clean — no blockers, no correctness bugs. The change is well-scoped, deterministically tested, and the prior review's log nit has been resolved.

Blocker

None.

Important

None.

Things I re-verified on this revision

• Log nit resolved. The latest commit (d8bdcfbff) splits the log into two branches: the auto-derive path logs the memory fields (availableMemoryMB, nodeMemoryReserveMB, maxMemoryPerCompilationMB), and the explicit-flag path logs only limit. This exactly addresses the earlier "misleading memory fields when overridden" feedback — the memory fields no longer imply they drove a value set by flag. 👍
• compilerInUse set is still complete. Traced every throttledCompiler consumer: the RPC handler (getCompiledCasm) is only served via makeRPCOverHTTP/makeRPCOverWebsocket, gated on cfg.HTTP/cfg.Websocket (node.go:566, 587); P2P passes it to p2p.New (node.go:464); the sequencer wires it (node.go:368). makeGRPC (node.go:629) takes only database, version — it does not compile — so gRPC is correctly excluded. cfg.HTTP || cfg.Websocket || cfg.P2P || cfg.Sequencer covers exactly the compiling paths.
• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to maxParallel, and availableMemory <= nodeMemoryReserve is guarded before the unsigned subtraction (concurrency.go:30–37).
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup (limit ≥ host) as host RAM, and FromCgroup() erroring on non-Linux falls back to host memory. Byte/MB units are consistent.
• Fail-fast + fallback correct. Returns a clear actionable error when budget is 0 and a compiling service is enabled; otherwise max(..., 1) keeps one slot for sync-only nodes. The two new node tests cover both branches deterministically (AvailableMemoryMB() + 4096 guarantees a zero budget regardless of host RAM).
• Builds & vets clean locally (starknet/compiler, node); table tests cover the boundary cases (==reserve, <reserve+one, cap-by-parallel, unbounded).

Nit (non-blocking)

• Codecov flags 2 uncovered lines in concurrency.go (the in-container return cgroupLimit / megabyte branch of AvailableMemoryMB). That path only runs under a real cgroup limit and is genuinely awkward to unit-test — fine to leave, but if you want the coverage, AvailableMemoryMB could be refactored to take the host/cgroup values as params so a pure-logic helper is testable. Optional.

Heads-up for release notes (not a bug)

On a many-core / memory-modest host (e.g. 16 CPU / 16 GB, defaults), derived concurrency drops from the old GOMAXPROCS (16) to (16-4)/4 = 3. Intended, but it's a meaningful default change for existing P2P / heavy getCompiledCasm deployments — worth a changelog line so operators aren't surprised by reduced compile parallelism.

[github-actions]

Claude finished @RafaelGranza's task in 57s —— View job

---

PR Review: memory-aware concurrency limit

• Read changed files
• Review starknet/compiler/concurrency.go (core logic)
• Review node/node.go wiring + fail-fast + log
• Review cmd/juno/juno.go flags + tests
• Build + vet affected packages
• Post review

This revision is clean — no blockers, no correctness bugs. Builds and vets clean locally (starknet/compiler, node, cmd). The change is well-scoped and deterministically tested.

Blocker

None.

Important

None.

Re-verified on this revision

• Log split is correct. node/node.go now logs "deriving Sierra compilation concurrency" with the memory fields (availableMemoryMB, nodeMemoryReserveMB, maxMemoryPerCompilationMB) only on the auto-derive path, and "using configured Sierra compilation concurrency" with just limit on the explicit-flag path. This resolves the earlier "misleading memory fields when overridden" feedback — the memory fields no longer imply they drove a value set by flag. 👍 (Answering your inline question: keep the log — it fires once at startup and is the line operators will grep when compilation is slower than expected; Info is the right altitude.)
• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to maxParallel, and availableMemory <= nodeMemoryReserve is guarded before the unsigned subtraction (concurrency.go:30–37).
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup (limit ≥ host) as host RAM; FromCgroup() erroring on non-Linux falls back to host memory. Byte/MB units are consistent.
• Fail-fast + fallback correct. Returns an actionable error when budget is 0 and a compiling service is enabled (cfg.HTTP || cfg.Websocket || cfg.P2P || cfg.Sequencer); otherwise max(..., 1) keeps one slot for sync-only nodes. The three node tests cover reject-on-zero-budget, ignore-when-no-compiler, and explicit-override — all deterministic (AvailableMemoryMB() + 4096 guarantees a zero budget regardless of host RAM).
• Queue derivation correct. MaxCompilationQueue == 0 → 2 * concurrentCompilations, applied after concurrency is resolved, so it also tracks an explicitly-set concurrency. Both flag defaults moved to 0 consistently in cmd/juno and the precedence test.

Nit (non-blocking)

• Codecov flags the in-container return cgroupLimit / megabyte branch of AvailableMemoryMB as uncovered. It only runs under a real cgroup limit and is awkward to unit-test — fine to leave. If you want the coverage, AvailableMemoryMB could take host/cgroup values as params so a pure-logic helper is testable. Optional.

Heads-up for release notes (not a bug)

On a many-core / memory-modest host (e.g. 16 CPU / 16 GB, defaults), derived concurrency drops from the old GOMAXPROCS (16) to (16-4)/4 = 3. Intended, but it's a meaningful default change for existing P2P / heavy getCompiledCasm deployments — worth a changelog line so operators aren't surprised by reduced compile parallelism.

[github-actions]

Claude finished @RafaelGranza's task in 3m 4s —— View job

---

PR Review: memory-aware concurrency limit (re-review after sync)

• Read changed files at latest commit
• Review starknet/compiler/concurrency.go (core logic)
• Review node/node.go wiring + log split
• Review cmd/juno/juno.go flags + tests
• Build + vet affected Go packages
• Post review

This revision is clean — no blockers, no correctness bugs. starknet/compiler and node build and go vet clean locally (the cmd/juno link failure here is only missing prebuilt Rust artifacts, unrelated to this change).

The notable change in this push is that the fail-fast path was replaced with a floor of 1 (max(ConcurrencyLimit(...), 1)), and the docs now state "The limit is always at least 1: on a memory-tight node compilations run one at a time rather than the node refusing to start." This is a sound simplification — a tight-memory node degrades to serial compilation instead of refusing to boot, and there's no longer any need to enumerate which services compile. The node_test.go cases were updated to match (TestNewNodeRunsOneAtATimeOnLowMemory now expects NoError).

Blocker

None.

Important

None.

Verified on this revision

• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to maxParallel, and availableMemory <= nodeMemoryReserve is guarded before the unsigned subtraction (concurrency.go:30–37). The floor of 1 also guarantees a usable compiler regardless of budget.
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup (limit ≥ host) as host RAM; FromCgroup() erroring on non-Linux falls back to host. Byte/MB units are consistent (memory.TotalMemory() and FromCgroup() are both bytes, divided by megabyte).
• Log split is correct. Auto-derive path logs the memory fields under "deriving Sierra compilation concurrency"; the explicit-flag path logs only limit under "using configured Sierra compilation concurrency" — the memory fields no longer imply they drove a flag-set value.
• Queue derivation correct. MaxCompilationQueue == 0 → 2 * concurrentCompilations, computed after concurrency resolves, so it tracks both derived and explicit concurrency.
• Tests deterministic. NodeMemoryReserve: AvailableMemoryMB() + 4096 forces a zero budget regardless of host RAM/OS, exercising the floor; TestNewNodeSkipsDerivedConcurrency covers the explicit-override branch. Flag defaults moved to 0 consistently across cmd/juno and the precedence test.
• Dependency claim holds — automemlimit → pbnjay/memory only; both move indirect → direct with no new transitive deps.

Nit (non-blocking)

• Stale PR description. The PR body still documents the fail-fast behavior ("the node fails fast at startup", the two startup error test rows). The code now floors at 1 and never refuses to start on a zero budget. Worth updating the description so it matches the shipped behavior. The docs (tuning.md) are already correct.
• Codecov: in-container return cgroupLimit / megabyte branch (concurrency.go:16) is uncovered — you noted this in an inline comment. It only runs under a real cgroup limit and is awkward to unit-test; fine to leave. If you want it, extracting a pure helper that takes (hostMemory, cgroupLimit, cgroupErr) would make the selection logic testable.

Heads-up for release notes (not a bug)

On a many-core / memory-modest host (e.g. 16 CPU / 16 GB, defaults), derived concurrency drops from the old GOMAXPROCS (16) to (16-4)/4 = 3. Intended, but a meaningful default change for existing P2P / heavy getCompiledCasm deployments — worth a changelog line so operators aren't surprised by reduced compile parallelism.
· branch granza/compiler-memory-limit

[github-actions]

Claude finished @RafaelGranza's task in 54s —— View job

---

PR Review: memory-aware concurrency limit (re-review on sync)

• Read changed files at latest commit
• Review starknet/compiler/concurrency.go (core logic)
• Review node/node.go wiring + log split
• Review cmd/juno/juno.go flags + tests
• Post review

This revision is clean — no blockers, no correctness bugs. The change is well-scoped and deterministically tested.

Blocker

None.

Important

None.

Verified on this revision

• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to maxParallel, and availableMemory <= nodeMemoryReserve is guarded before the unsigned subtraction (concurrency.go:30–37). max(ConcurrencyLimit(...), 1) in node.go:296 guarantees a usable compiler even on a zero budget, so a memory-tight node degrades to serial compilation rather than refusing to boot.
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup (limit ≥ host) as host RAM; FromCgroup() erroring on non-Linux falls back to host. Units consistent — both memory.TotalMemory() and FromCgroup() are bytes, divided by megabyte.
• Log split correct. Auto-derive path logs the memory fields under "deriving Sierra compilation concurrency"; the explicit-flag path logs only limit under "using configured Sierra compilation concurrency" — memory fields no longer imply they drove a flag-set value. This resolves your earlier inline question.
• Queue derivation correct. MaxCompilationQueue == 0 → 2 * concurrentCompilations, computed after concurrency resolves, so it tracks both derived and explicit concurrency.
• Tests deterministic. NodeMemoryReserve: AvailableMemoryMB() + 4096 forces a zero budget regardless of host RAM/OS, exercising the floor (TestNewNodeRunsOneAtATimeOnLowMemory); TestNewNodeSkipsDerivedConcurrency covers the explicit-override branch. Flag defaults moved to 0 consistently across cmd/juno and the precedence test. Table tests cover the boundary cases (==reserve, <reserve+one, cap-by-parallel, unbounded).
• Dependency claim holds — automemlimit's only dep is pbnjay/memory; both move indirect → direct with no new transitive deps (go.mod).

Nit (non-blocking)

• Codecov: in-container return cgroupLimit / megabyte branch (concurrency.go:16) is uncovered — you flagged this inline yourself. It only runs under a real cgroup limit and is awkward to unit-test; fine to leave. If you want it, extract a pure helper taking (hostMemory, cgroupLimit, cgroupErr) so the selection logic is testable in isolation.

Heads-up for release notes (not a bug)

On a many-core / memory-modest host (e.g. 16 CPU / 16 GB, defaults), derived concurrency drops from the old GOMAXPROCS (16) to (16-4)/4 = 3. Intended, but a meaningful default change for existing P2P / heavy getCompiledCasm deployments — worth a changelog line so operators aren't surprised by reduced compile parallelism.
· branch granza/compiler-memory-limit

[rodrodros]

Rafael the 0 as a way to set is default is not a good solution because 0 can be a desired value in itself. A node set with 0,0 values can me a node that will reject any compilation requests.

Since Cobra allow us to have some expression, can't the defaults be triggered at runtime and CLI print the default values? I don't know if this is a good approach.

The other approach is to allow other values, such as "default" or "dynamic" which are non-numbers and if set, then the calculation is done

[RafaelGranza]

@rodrodros
I Switched the default to "" (empty string), so 0 stays valid as "disable". So the flags are now Int. It is the simplest option I found, but open to alternatives.

[github-actions]

Claude finished @RafaelGranza's task in 3m 52s —— View job

---

PR Re-review: -1 sentinel for derived concurrency

Reviewed the switch from uint+0-sentinel to int+-1-sentinel (commit c3a7e8ee3). Built starknet/compiler and node clean.

Verdict: the -1 approach is sound and correctly resolves @rodrodros's concern. No blockers, no correctness bugs.

Answering the design question

Switched the default to -1, so 0 stays valid as "disable". Open to alternatives.

This is the right call, and I'd keep it over a string "default"/"dynamic" enum — int keeps the flag type simple, avoids custom parsing, and -1 reads naturally as "unset/auto". I verified the 0 semantics @rodrodros wanted actually hold end-to-end:

• With concurrency=0, queue=0, the throttler computes maxRequests = 0 + 0 = 0, and Throttler.Do returns ErrResourceBusy for every request (activeReqs (1) > maxRequests (0)). So --max-concurrent-compilations 0 is a genuine "reject all compilations" node, exactly as intended. ✅
• The floor of 1 (max(ConcurrencyLimit(...), 1)) only applies on the derive path, so explicit 0 is not silently bumped to 1. Correct — the floor is a memory-budget guard, not an override of an explicit operator choice. ✅
• Non-Linux: PreRunE zeroes MaxCompilationMemory when unset → derive path hits the maxMemoryPerCompilation == 0 short-circuit → returns GOMAXPROCS. Matches the old default and the tuning.md wording. ✅
• MaxCompilationQueue == -1 → 2 * concurrentCompilations, tracking both derived and explicit concurrency. ✅
• Field type change uint → int: no other consumers — only node.go and cmd/juno reference these fields, and both convert explicitly. ✅

Nits (non-blocking)

• Magic -1 / < 0 checks. node/node.go:299 and :320 test < 0, and the sentinel -1 is repeated as a literal in cmd/juno/juno.go:490-491. This also ties into @rodrodros's "maybe they can be constants?" comment — a single shared const deriveFromMemory = -1 (or similar) used in both the flag default and the node.go branch would make intent self-documenting and keep the two sites from drifting. Optional but cheap.
• All negatives derive, not just -1. < 0 means -5 also triggers derive, while the help text documents only -1. Harmless, but if you want to be strict you could treat only -1 as the sentinel and reject other negatives in PreRunE. Low priority.
• Stale PR description. The body still describes the old fail-fast behavior and the 0-sentinel flag table (--max-concurrent-compilations default 0). The shipped behavior is -1 default + floor-to-1. Worth syncing the description; docs/tuning.md is already correct.
• (Pre-existing) Codecov still flags the in-container return cgroupLimit / megabyte branch (concurrency.go:16) — awkward to unit-test, fine to leave.

Heads-up for release notes (not a bug)

Default derived concurrency on a many-core/memory-modest host (e.g. 16 CPU / 16 GB) is (16-4)/4 = 3, down from the old GOMAXPROCS (16). Intended, but a meaningful default change for heavy P2P / getCompiledCasm deployments — worth a changelog line.
· branch granza/compiler-memory-limit

[github-actions]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[github-actions]

Claude finished @RafaelGranza's task in 3m 22s —— View job

---

PR Re-review (sync)

• Read changed files at latest commit (33ce1f5fa)
• Review starknet/compiler/concurrency.go + compiler.go
• Review node/throttled_compiler.go + node/node.go
• Review cmd/juno/juno.go flags + tests
• Build + go vet affected packages (clean)

Verdict: clean — no blockers, no correctness bugs. node and starknet/compiler build and go vet clean locally. This revision settles on the uint64-flags + per-flag *Explicit design, which is the most idiomatic answer yet to @rodrodros's "0 is a desired value" objection, and every item from the CHANGES_REQUESTED round is addressed.

@rodrodros's CHANGES_REQUESTED items — all addressed

• Live in throttled_compiler.go ✅ Derivation, settings, and construction are now in node/throttled_compiler.go; New() calls newThrottledCompilerFromConfig(cfg, logger) in one line (node.go:302), so the oversized New() no longer grows.
• uint64 instead of string parsing ✅ Config.MaxConcurrentCompilations / MaxCompilationQueue are uint64; the Cobra flags are Uint64, so negatives / non-numeric are rejected by the flag parser. The hand-rolled parseCompilationLimit (and the old CodeQL Fix shadowed declaration #766 uint64→uint site) is gone.
• Naming ✅ concurrentAuto→maxConcurrencyDerived, concurrent→maxConcurrency, queue var→queueSize; concurrency.go's parameter is maxConcurrency (no more "parallel" disparity).
• Reuse the variable / no unnecessary assign ✅ maxConcurrency and queueSize are each declared once from the config and reassigned in place on the derive branch.
• "A test please" ✅ The former error branch (parseCompilationLimit) no longer exists — invalid input is now rejected by Cobra's Uint64 parser, so the branch that needed a test is designed out. TestNewNodeRunsOneAtATimeOnLowMemory covers derive-and-floor, TestNewNodeSkipsDerivedConcurrency the explicit path, and TestConfigPrecedence + the *Explicit detection table cover source-agnostic detection.

Verified correct on this revision

• 0 vs unset end-to-end. Unset → !Explicit → derive (floored to 1 inside ConcurrencyLimit); explicit 0 → Explicit=true → used as-is → throttler maxRequests = 0 → every request ErrResourceBusy. The floor is only reached on the derive path, so an explicit 0 is never bumped to 1. ✅
• Explicit is source-agnostic. viper.IsSet is true for CLI and config-file/env, false for an unchanged default — matched by the "numeric compilation limits in config file" precedence case and the per-flag *Explicit table. ✅
• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to max(1, maxConcurrency); availableMemory <= nodeMemoryReserve is guarded before the unsigned subtraction (concurrency.go:30-36). ✅
• Queue tracks concurrency. !MaxCompilationQueueExplicit → 2 * maxConcurrency, computed after concurrency resolves (throttled_compiler.go:42-45). ✅
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup as host RAM; FromCgroup() errors fall back to host; byte→MB units consistent. ✅
• GOMAXPROCS pin (compiler.go) unchanged and still correct — appended after os.Environ(), so os/exec dedup keeps the pinned value.
• Log casing fixed ✅ (your reply) — availableMemoryMB / nodeMemoryReserveMB / maxCompilationMemoryMB now consistent camelCase + MB. The maxConcurrencyDerived / queueSizeDerived bools disambiguate whether memory drove the value, which resolves the original "misleading memory fields when overridden" concern in a single line.

Nits (non-blocking)

• throttled_compiler.go:32 — AvailableMemoryMB() is called unconditionally, even on the explicit path where it's only used for the log line. It's a cheap cgroup read at startup, so harmless, but you could move it inside the if !…Explicit block (and drop it from the log on the explicit path) if you want the call to happen only when it actually feeds a decision. Optional.
• cmd/juno/juno.go:276 — usage const maxCompilationReserveMemory breaks the <flag>Usage naming convention. You said you'll leave it as is — noting only for completeness.
• throttled_compiler.go:86 — uint(settings.maxConcurrency) narrows uint64→uint; lossless on the 64-bit targets Juno ships. Fine to leave since NewThrottledCompiler takes uint.

Heads-up for release notes (not a bug)

Default derived concurrency on a many-core / memory-modest host (e.g. 16 CPU / 16 GB) is (16-4)/4 = 3, down from the old GOMAXPROCS. Intended, but a meaningful default change for heavy P2P / getCompiledCasm deployments — worth a changelog line.
· branch granza/compiler-memory-limit

[github-actions]

Claude finished @RafaelGranza's task in 1m 22s —— View job

---

PR Re-review (sync)

• Read changed files at latest commit
• Review starknet/compiler/concurrency.go + compiler.go
• Review node/throttled_compiler.go + node/node.go
• Review cmd/juno/juno.go flags + tests
• Build + go vet affected packages (clean)

Verdict: clean — no blockers, no correctness bugs. node and starknet/compiler build and go vet clean locally. This revision settles on the uint64-flags + per-flag *Explicit bool design, and every item from @rodrodros's CHANGES_REQUESTED round is addressed.

Verified correct on this revision

• 0 vs unset end-to-end. Unset → !Explicit → derive; explicit 0 → used as-is → throttler maxRequests = 0 → every request ErrResourceBusy. The floor-to-1 now lives inside ConcurrencyLimit (concurrency.go:31,35,40) and is only reached on the derive path (throttled_compiler.go:33-40), so an explicit 0 is never bumped to 1. ✅
• Explicit is source-agnostic. v.IsSet(...) (cmd/juno/juno.go:436-437) is true for CLI and config-file/env, false for an unchanged default. ✅
• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to max(1, maxConcurrency); availableMemory <= nodeMemoryReserve is guarded before the unsigned subtraction (concurrency.go:30-36). ✅
• Lossless conversion. Flags are Uint64, so negatives / non-numeric input are rejected by Cobra's parser (the old hand-rolled parseCompilationLimit/CodeQL Fix shadowed declaration #766 site is gone). uint(settings.maxConcurrency) at throttled_compiler.go:86 is lossless on the 64-bit targets Juno ships. ✅
• Queue tracks concurrency. !MaxCompilationQueueExplicit → 2 * maxConcurrency, computed after concurrency resolves (throttled_compiler.go:42-45). ✅
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup as host RAM; FromCgroup() errors fall back to host; byte→MB units consistent (concurrency.go:12-19). ✅
• GOMAXPROCS pin (compiler.go:112) unchanged and correct — appended after os.Environ(), so os/exec dedup keeps the pinned value even if the operator exports GOMAXPROCS.
• Log casing fixed (throttled_compiler.go:47-55) — consistent camelCase + MB suffix; the maxConcurrencyExplicit/queueSizeExplicit bools disambiguate whether memory drove the value.
• Tests deterministic — NodeMemoryReserve: AvailableMemoryMB() + 4096 forces a zero budget regardless of host (TestNewNodeRunsOneAtATimeOnLowMemory); TestNewNodeSkipsDerivedConcurrency / TestNewNodeSkipsDerivedQueue cover the explicit paths; the concurrency table covers ==reserve, <reserve+one, cap-by-parallel, and unbounded.

Nits (non-blocking)

• throttled_compiler.go:32 — AvailableMemoryMB() is called unconditionally, including on the explicit path where it only feeds the log line. Cheap cgroup read at startup, so harmless; could move inside the if !…Explicit block if you want it to run only when it drives a decision. Optional.
• cmd/juno/juno.go:276 — usage const maxCompilationReserveMemory breaks the <flag>Usage naming convention its neighbours follow. You said you'll leave it — noting only for completeness.

Heads-up for release notes (not a bug)

Default derived concurrency on a many-core / memory-modest host (e.g. 16 CPU / 16 GB) is (16-4)/4 = 3, down from the old GOMAXPROCS. Intended, but a meaningful default change for heavy P2P / getCompiledCasm deployments — worth a changelog line.
· branch granza/compiler-memory-limit

[RafaelGranza]

@rodrodros I refactored the code.
The flags are now uint64 and parsed at the cmd level: if you don't pass them, we derive the values.
The throttled compiler setup lives entirely in its own file now, so node.New here is cleaner than the one in main.

I decided to parse the flags at the cmd level because otherwise the throttled compiler, which was error-free, would suddenly have to carry parsing errors. On top of that, parsing a CLI flag shouldn't live inside throttled_compiler.go or node.New.

[rodrodros]

You are creating a struct (also public) to handle just two values, which is also used only one? Couldn't this function return the two appointed values directly?

[RafaelGranza]

It was to keep these two tied together, so that resolving the setting would actually return a setting.
I wasn't aware structs could be lowercase (private), which it should be.

I'm not sure if it fits as over-engineering, but it is not needed.

[RafaelGranza]

i´ll be removing it anyway

[rodrodros]

What's the point of NewThrottledCompiler now? It seems to me there is no longer a point for this function?

[RafaelGranza]

I like the test that NewThrottledCompiler exercises (node/throttled_compiler_test.go).
It could also be exercised by the new functions, but we would require us to set them public and passing a compiler.Compiler, creating a messier signature,.
More than that, i think it is good having a public log-free constructor.

[RafaelGranza]

I considered computing maxConcurrency internally, but it was handy for testing its behavior.

[github-actions]

Claude finished @RafaelGranza's task in 3m 54s —— View job

---

PR Re-review (sync): cmd-level uint64 parsing

• Read changed files at latest commit
• Review starknet/compiler/concurrency.go + compiler.go
• Review node/throttled_compiler.go + node/node.go
• Review cmd/juno/juno.go flags + tests + docs
• go vet ./starknet/compiler/ ./node/ — clean

Verdict: clean — no blockers, no correctness bugs. This sync is the refactor you described: flags are uint64 parsed by Cobra, derivation lives in node/throttled_compiler.go, and node.New is a one-line call (newThrottledCompilerFromConfig). The design reads well and settles the long thread with @rodrodros.

Verified on this revision

• 0 vs unset end-to-end. Unset → !…Explicit → derive; explicit 0 (CLI/env/YAML) → used as-is. NewThrottledCompiler(res, 0, 0) gives the throttler maxRequests = 0, and Do returns ErrResourceBusy for every request before touching the zero-cap semaphore (throttler.go:70), so a 0,0 node rejects all compilations without deadlocking. ✅
• Floor only on the derive path. The max(1, …) now lives inside ConcurrencyLimit (concurrency.go:31,35,40) and is only reached when !MaxConcurrentCompilationsExplicit, so an explicit 0 is never silently bumped to 1. ✅
• Parsing moved to the flag layer is the right call. Uint64 flags reject negatives / non-numeric at parse time, so resolveThrottledCompilerSettings is genuinely error-free — no more hand-rolled parseCompilationLimit and the old CodeQL Fix shadowed declaration #766 uint64→uint site is gone. uint(maxConcurrency) at throttled_compiler.go:77 is lossless on the 64-bit targets Juno ships. ✅
• Explicit is source-agnostic. v.IsSet(...) (cmd/juno/juno.go:435-436) is true for CLI and config-file/env, false for an unchanged default — matched by TestCompilationLimitExplicitDetection and the new numeric compilation limits in config file / via CLI precedence cases. ✅
• No underflow / divide-by-zero. maxMemoryPerCompilation == 0 short-circuits to max(1, maxConcurrency); availableMemory <= nodeMemoryReserve guarded before the unsigned subtraction (concurrency.go:30-36). Non-Linux path zeroes MaxCompilationMemory when unset (juno.go:425-427), so it hits that short-circuit and yields GOMAXPROCS — matching the old default. ✅
• cgroup logic sound. cgroupLimit > 0 && cgroupLimit < hostMemory treats an "unlimited" cgroup as host RAM; FromCgroup() errors fall back to host; byte→MB units consistent. ✅
• Queue tracks concurrency. !MaxCompilationQueueExplicit → 2 * maxConcurrency, computed after concurrency resolves. ✅
• GOMAXPROCS pin (compiler.go:112) unchanged and correct — appended after os.Environ(), so os/exec dedup keeps the pinned value even if the operator exports GOMAXPROCS.
• Docs updated. tuning.md and generate-config.js now describe the derive formula, the empty-default semantics, 0 = disable, cgroup behavior, and the floor-to-1 — matching the shipped code, and resolving @rodrodros's generate-config.js comment.
• Log casing consistent (throttled_compiler.go:42-48) — camelCase + MB suffix; the Explicit log fields were dropped as @rodrodros asked.

Nits (non-blocking)

• throttled_compiler.go:27 — AvailableMemoryMB() is called unconditionally, including on the explicit path where it only feeds the log line. It's a cheap startup cgroup read, so harmless, but it could move inside the if !…Explicit block if you'd rather the read only happen when it drives a decision. Optional.
• cmd/juno/juno.go:276 — usage const maxCompilationReserveMemory breaks the <flag>Usage naming convention its neighbours follow. You said you'll leave it — noting only for completeness.
• docs/docs/tuning.md:107 — minor typo: "on a memory-tight node compilations, it runs one at a time" reads awkwardly (stray comma / plural). Cosmetic.

Heads-up for release notes (not a bug)

Default derived concurrency on a many-core / memory-modest host (e.g. 16 CPU / 16 GB) is (16-4)/4 = 3, down from the old GOMAXPROCS. Intended, but a meaningful default change for heavy P2P / getCompiledCasm deployments — worth a changelog line.
· branch granza/compiler-memory-limit