Skip to content

keccak: test that the permutation kernel stays preemptible - #33

Closed
AskAlexSharov wants to merge 2 commits into
masterfrom
test/asm-preemption
Closed

AskAlexSharov wants to merge 2 commits into
masterfrom
test/asm-preemption

Conversation

@AskAlexSharov

Copy link
Copy Markdown
Contributor

#32 removed NOSPLIT from the amd64 kernel so the runtime regains a preemption point in the absorb and squeeze loops. Nothing stops it coming back, and the failure is invisible in throughput benchmarks — it only shows up as a stop-the-world pause in whatever else the process is doing.

TestKernelKeepsStackCheck

Reads the TEXT directives and rejects both ways of losing the check:

  • NOSPLIT, which drops it whatever the frame size;
  • a frame under 128 bytes, because the assembler marks a leaf that small NOSPLIT on its own (abi.StackSmall, arm64).

The second one is the trap: deleting the keyword alone changes nothing if the frame is small, which is why #12 would have reintroduced the stall on arm64. The test reads both .s files regardless of the runner's architecture, so an arm64 CI job still catches an amd64 regression.

TestSTWPauseWhileHashing

Measures the property the directives only imply, using the runtime's own /sched/pauses/stopping/gc:seconds histogram while hashing 64 MiB with GC pressure on another goroutine. It reads the histogram delta rather than the process-lifetime max, and waits before reading so a blocked stop-the-world is recorded first.

Verification

Both tests pass on master. Putting NOSPLIT back on the kernel of each architecture:

now with NOSPLIT restored
amd64, EPYC 4344P 20.48 µs 83.886 ms
arm64, M4 Max 114.69 µs 58.720 ms

The budget is 5 ms — two orders of magnitude away from both the healthy and the broken value, so there is nothing to tune. The test takes 1.3 s and skips under -short, when GOMAXPROCS < 2, and when the CPU has no assembly backend (the pure-Go path is preemptible, so there is nothing to measure).

#32 removed NOSPLIT from the amd64 kernel so the runtime regains a preemption
point in the absorb and squeeze loops. Nothing stops it coming back, and the
failure is invisible in throughput benchmarks: it only shows up as a
stop-the-world pause in whatever else the process is doing.

TestKernelKeepsStackCheck reads the TEXT directives and rejects both ways of
losing the stack-growth check: NOSPLIT, and a frame under 128 bytes, which
makes the assembler mark a leaf NOSPLIT on its own. It covers both
architectures from any runner, so an arm64 CI job still catches an amd64
regression.

TestSTWPauseWhileHashing measures the property itself from the runtime's
/sched/pauses/stopping/gc:seconds histogram. Its budget is relative: it hashes
the same buffer through the pure-Go fallback first, which always has a
preemption point, and only fails if the assembly path stalls the world four
times longer than that and past 5ms. A slow or loaded runner moves both numbers
together; only a real regression separates them.

Verified by putting NOSPLIT back, assembly against the pure-Go floor:

    amd64 (EPYC 4344P)   20.48 us -> 83.886 ms
    arm64 (M4 Max)      327.68 us -> 100.663 ms, floor 163.84 us
…togram

The stopping-pause histogram is cumulative for the life of the process, so
picking its highest non-empty bucket attributes any earlier pause -- another
test, the warm-up -- to the code under test. Taking bucket deltas fixes that but
needs a sleep afterwards so a blocked stop-the-world is recorded before the
snapshot, which is easy to get wrong.

Timing runtime.GC() from another goroutine measures the same thing without a
histogram, and the budget is already relative to the pure-Go fallback, so the
extra precision was not buying anything.

Verified both ways on both architectures. Healthy: 327us assembly against 668us
pure Go on M4 Max, 416us against 526us on EPYC 4344P. With NOSPLIT put back:
91.9ms against 1.1ms, and 73.8ms against 596us.

Matches the same test in erigon and klauspost/compress.
@AskAlexSharov

Copy link
Copy Markdown
Contributor Author

Merged into #35. The test is red on master because keccakF1600BMI2 is still NOSPLIT there, so it only goes green together with that fix — keeping them in one PR avoids landing a knowingly-failing test. Closing in favour of #35.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant