Repository navigation
perf: watch the call's context with AfterFunc instead of a goroutine - #39
Merged
Merged
Conversation
Every existing benchmark calls with context.Background, which skips the cancellation watchdog, so its cost never showed up although nearly all real request contexts are cancellable. The new test pins down that the watchdog is released once a call returns: servers cancel each request's context after the handler, and a still registered watchdog would pile up on long-lived contexts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Calls with a cancellable context, i.e. nearly every request, started a
goroutine and a child context per call just to notice a cancellation
early. A CPU profile showed starting, scheduling and tearing down that
goroutine taking most of each call, and the child context most of the
rest. context.AfterFunc registers a callback instead and only starts a
goroutine if the context actually gets canceled during the call, so
cancellations are still recorded as promptly. The internal cancellation
cause existed only to tell the goroutine's own wake-up apart from a real
cancellation and goes away with it.
benchstat, n=10, main and this commit's test binaries run interleaved:
./main.test|./new.test -test.run '^$' -test.bench BenchmarkHoglet_Do_ \
-test.benchmem -test.benchtime 1s -test.cpu 1,8,24 -test.count 1
Hoglet_Do_EWMA_cancellable 1471.0n 513.6n -65.08% (p=0.000)
Hoglet_Do_EWMA_cancellable-8 609.6n 127.5n -79.08% (p=0.000)
Hoglet_Do_EWMA_cancellable-24 455.1n 122.3n -73.14% (p=0.000)
Still 4 allocs/op, 177 -> 224 B/op. Benchmarks with an uncancellable
context are unchanged within noise.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous test only noticed a leaked watchdog because the callback consults the failure condition before the deduplicated observe, and its state assertion could never fail since the dedup had already fired. A leaked watchdog that decided inside the dedup passed it. A context's AfterFunc method, which context.AfterFunc documents as its registration hook, counts registrations and stops directly, without synctest or a cancellation; both mutants now fail it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A closure created inside the generic Wrap is allocated one size class
bigger than the same closure in a plain method (64 instead of 48 bytes,
per memory profile), so the callback moves into Circuit.watch, which is
still inlined. The two comment blocks in Wrap that repeated each other
and Wrap's doc comment are trimmed with it.
benchstat, n=10, previous and this commit's test binaries run interleaved:
./prev.test|./new.test -test.run '^$' -test.bench BenchmarkHoglet_Do_ \
-test.benchmem -test.benchtime 1s -test.cpu 1,8,24 -test.count 1
Hoglet_Do_EWMA_cancellable 508.9n 508.2n ~ (p=0.617)
Hoglet_Do_EWMA_cancellable-8 133.1n 125.9n -5.41% (p=0.001)
Hoglet_Do_EWMA_cancellable-24 125.6n 110.5n -12.02% (p=0.000)
Still 4 allocs/op, 224 -> 208 B/op.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The comment named ErrConcurrencyLimit, which does not exist; the sentinel is ErrConcurrencyLimitReached. Bracketing both names makes them doc links, so renames are caught. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
ErrConcurrencyLimitReached is what a non-blocking limiter returns, not a blocking one; a blocking limiter returns ErrWaitingForSlot, which the comment described without naming. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused optimization preserves cancellation behavior and includes appropriate performance and cleanup coverage.
0 open findings
What changed in this PR
Optimizes context cancellation monitoring by replacing per-call watchdog goroutines with context.AfterFunc.
Changes:
- Registers and unregisters cancellation callbacks efficiently.
- Adds cancellable-context benchmarking and cleanup coverage.
- Corrects
Wraperror documentation.
| File | Description |
|---|---|
hoglet.go |
Implements the AfterFunc watchdog and updates documentation. |
hoglet_test.go |
Adds benchmark and watchdog unregistration test. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
costela
marked this pull request as ready for review
October 9, 2026 17:10
vietuoso
approved these changes
Oct 12, 2026
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
Calls with a cancellable context, which is nearly every request, started a goroutine and a child context per call just to notice a cancellation early. On main such a call costs about 51x a call with
context.Background()on one CPU, and about 176x and 258x on 8 and 24 CPUs (table below). No benchmark showed it, since all of them usedcontext.Background(), which skips the watchdog.What
context.AfterFunc, which only starts a goroutine if the context is actually canceled during the call. Cancellations are still recorded as promptly; the internal cancellation cause andobserveCtxgo away. The callback is created in a small non-generic method, since a closure created inside the genericWrapis allocated one size class bigger.AfterFuncmethod, the hookcontext.AfterFuncdocuments.Wrap's comment on unobserved errors: it named a nonexistentErrConcurrencyLimitand attributed the non-blocking limiter's error to blocking circuits. The sentinels are now doc links.benchstat, n=10, main and this branch's test binaries run interleaved:
Hoglet_Do_EWMA(context.Background())Hoglet_Do_EWMA-8Hoglet_Do_EWMA-24Hoglet_Do_EWMA_cancellableHoglet_Do_EWMA_cancellable-8Hoglet_Do_EWMA_cancellable-24Still 4 allocs/op, ~177 → 208 B/op. The
context.Background()path is untouched by this change, so its ±1.5% marks the noise floor. The existing sliding-window benchmarks also call withcontext.Background(), which never starts a watchdog, so they show no change either. The watchdog depends on the caller's context, not on the breaker: a sliding window called with a cancellable context (scratch benchmark in the same run) goes from 1458.5 ns to 598.5 ns on 1 CPU (-58.96%), 580.2 ns to 173.9 ns on 8 (-70.03%) and 504.2 ns to 179.0 ns on 24 (-64.49%).🤖 Generated with Claude Code