Skip to content

cmd/utils: tune GOGC for large cache values#34851

Merged
jrhea merged 4 commits into
ethereum:masterfrom
s1na:opt/hawk-iso
Jul 14, 2026
Merged

cmd/utils: tune GOGC for large cache values#34851
jrhea merged 4 commits into
ethereum:masterfrom
s1na:opt/hawk-iso

Conversation

@s1na

@s1na s1na commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Needs bench with different cache values.

● GC Memory-Limit Retune - engine_newPayload benchmark
  ===========================================================
  Commit before: a06558042 (master)
  Commit after:  feea0dab1 (cmd/utils: tune GOGC for large cache values)
  Blocks: 1k mainnet (24950066 → 24951065)
  Runs: 3 baseline / 2 hawk, clean ZFS clone per run

                  Before (avg)    With Hawk (avg)     Δ
  Throughput      176.0 MGas/s    184.7 MGas/s        +4.9%
  Mean NP         172.3ms         164.2ms             -4.7%
  p50             162.8ms         156.7ms             -3.8%
  p95             282.4ms         270.4ms             -4.3%
  p99             391.0ms         373.8ms             -4.4%

  Machine: Intel Ultra 7 255H, 62GB DDR5, NVMe (ZFS), governor=performance, turbo=off

Comment thread cmd/utils/flags.go Outdated
memLimit = int64(cache) * 1024 * 1024 * 2
}
log.Debug("Setting Go memory limit", "MB", memLimit/1024/1024)
godebug.SetMemoryLimit(memLimit)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit dangerous to use SetMemoryLimit. If the memory usage approaches to this limit, the GC will be very aggressive and the CPU overhead will be insane.

In general, we need to be aware that what's the theoretical memory limit we will have.

@s1na

s1na commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Feedback from triage: 1) introduce --memorylimit flag. 2) if machine RAM can't be determined don't touch defaults.

Comment thread cmd/utils/flags.go Outdated
// processes; cap the rest at roughly 4× the configured DB cache
// to avoid runaway growth on machines with lots of RAM but a
// modest --cache.
halfHost := int64(mem.Total / 2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mem.Total here is the host's physical RAM, not this process's cgroup limit.
#34947 already replaced gopsutil.VirtualMemory() with the cgroup-aware memlimit.Limit(), but this PR predates it. i think we just need to rebase from master and use total returned from memlimit.Limit().

Comment thread cmd/utils/flags.go Outdated
// Fallback when the host's total memory isn't reportable.
memLimit = int64(cache) * 4 * 1024 * 1024
}
if memLimit < int64(cache)*1024*1024*2 {

@jrhea jrhea Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check can override the total/2 cap above it and let the Go memory limit climb to two-thirds of total memory.

this check triggers when:

memLimit < 2*cache

and we know that:

memLimit = min(total/2, 4*cache)

so that's:

min(total/2, 4*cache) < 2*cache

that results in 2 cases and this one is never true:

4*cache < 2*cache

so we're left with:

total/2 < 2*cache

cache is clamped above to <= total/3, so plugging in that maximum:

total/2 < 2*(total/3)

which simplifies to:

3*total < 4*total

which is always true. so at the largest cache allowed, this check fires 
(in general, whenever cache > total/4) and overrides the cap to:

memLimit = 2*cache = 2*total/3

which is above the total/2 we meant to cap at. so instead of leaving half of memory for the OS, page cache, and pebble, we let the Go heap grow to two-thirds.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UPDATE: I removed this code block in the updates I pushed

@jrhea jrhea marked this pull request as ready for review July 9, 2026 21:59
@jrhea

jrhea commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Bench: fork pointpr/34851

2k payload · blocks 24950076–24952065 · 3 runs/side · machine joey
ran 2026-07-09 21:51 UTC – 2026-07-09 22:24 UTC

Paired per-block newPayload (n=1990): median -9.1% · trimmed mean -9.7% · 81% of blocks faster — each block vs itself, lower noise than the aggregate below

Summary

metric base (fork point) target (pr/34851) Δ
throughput MGas/s 236.2 (±0.3%) 264.8 (±0.4%) +12.1%
mean newPayload 127.7 ms (±0.3%) 113.9 ms (±0.4%) -10.8%
p50 newPayload 116.4 ms (±0.5%) 104.9 ms (±0.3%) -9.9%
p95 newPayload 217.2 ms (±1.5%) 188.6 ms (±0.8%) -13.2%
p99 newPayload 314.3 ms (±1.3%) 285.7 ms (±2.0%) -9.1%

▲ improvement · ▼ regression · ≈ within run-to-run noise

Per-run (3 runs/side)

side run MGas/s mean p50 p95 p99 min max total gas
base 1 236.5 127.5 ms 115.8 ms 217.2 ms 315.8 ms 18.4 ms 1698.7 ms 60.0 Ggas
base 2 235.2 128.2 ms 117.0 ms 220.5 ms 317.6 ms 10.7 ms 1666.7 ms 60.0 Ggas
base 3 236.8 127.3 ms 116.4 ms 213.9 ms 309.5 ms 16.0 ms 1562.5 ms 60.0 Ggas
target 1 264.0 114.2 ms 104.9 ms 188.5 ms 289.8 ms 10.7 ms 1539.2 ms 60.0 Ggas
target 2 264.1 114.2 ms 105.3 ms 187.2 ms 288.9 ms 10.3 ms 1502.5 ms 60.0 Ggas
target 3 266.4 113.2 ms 104.6 ms 190.1 ms 278.5 ms 10.3 ms 1538.9 ms 60.0 Ggas

Execution breakdown (avg per block)

metric base target
slow-log blocks 1990 1990
execution 64.2 ms 60.9 ms
state read 20.5 ms 16.4 ms
state hash 9.6 ms 7.1 ms
commit 14.9 ms 12.2 ms
total 111.3 ms 98.5 ms
account cache hits 90.6% 91.6%
storage cache hits 85.7% 86.9%

Comment thread cmd/utils/flags.go Outdated
Comment on lines +1786 to +1792
cache := int64(ctx.Int(CacheFlag.Name)) * 1024 * 1024
// Target ~4x the cache so GOGC=100 governs steady state, but cap at
// half of memory. The rest is for the OS, page cache, and off-heap
// caches like Pebble that SetMemoryLimit doesn't count.
memLimit := min(int64(total)/2, cache*4)
log.Debug("Setting Go memory limit", "MB", memLimit/1024/1024, "source", source)
godebug.SetMemoryLimit(memLimit)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the automatic setting of memory limit

@jrhea jrhea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jrhea jrhea added this to the 1.17.5 milestone Jul 14, 2026
@jrhea jrhea merged commit c3f2851 into ethereum:master Jul 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants