perf + cleanup: production-ready go-pubsub (PR-1)#2
Merged
Conversation
Expands the public-surface test coverage, fixes a vacuous test, and adds project documentation. Verified with go test -race -count=3 ./pubsub/... (all 31 tests + 7 examples + 16 benchmarks green) and a red-green regression check on the rewritten test. Adds to pubsub/: - example_test.go: 7 godoc Example* tests covering the basic flows and documenting the ExampleType_Method naming requirement. - coverage_test.go: 16 unit tests filling gaps in the public surface - WithLogger/WithId validation errors, all Id/String accessors, Subscribe-after-Close, double Close, multi-topic Close, sliding timeout reset on activity, Publish auto-create, capacity-exceeded, struct payload, Subscribes partial-failure, concurrent stress. - bench_test.go: 4 new benchmarks (Subscribes, BrokerTopics, StructPayload, PublishAutoCreateTopic). Fixes pubsub_test.go: - TestMultiPublisherMultipleSubscribers was passing vacuously: it closed each subscription before publishing, then read zero values from the closed channels without checking the ok flag. The receive case now checks ok and breaks on close. Red-green verified: reintroducing the bug causes the test to fail with "expected 15 messages, received 0". Adds project context: - CLAUDE.md: project overview, commands, architecture, locking pattern, capacity model, testing conventions, gotchas, non-goals. - .claude/rules/git-commits.md: English-only Conventional Commits, Co-Authored-By with current model name only (no email). - .claude/rules/code-style.md: English for exported API and all user-visible strings; Chinese for internal comments. Co-Authored-By: MiniMax-M3
The .gitignore was the GitHub Go community template with a few entries that were only locally excluded (Claude Code scratch); rewrite it into a sectioned form so future contributors know where each rule belongs. Add .gitattributes that forces LF on every text file so Windows checkouts with core.autocrlf=true cannot accidentally commit CRLF, and explicitly marks common binary formats as binary so Git's auto-detection can never misclassify them. Also fix gofmt on pubsub/bench_test.go (missing final newline). Co-Authored-By: MiniMax-M3
Add a package-level godoc comment in pubsub.go and per-identifier godoc on every exported func/type/var/const in broker.go, publisher.go, and subscriber.go. Without these, pkg.go.dev rendered the package as a bare list of identifiers. Replace two bare errors.New validations in broker.go (WithLogger nil check, WithId empty check) with the new sentinels ErrLoggerNil and ErrBrokerIdEmpty so callers can match them via errors.Is. Translate the slog lock-trace and timeout messages from Chinese to English to match CLAUDE.md's user-visible-string rule (slog text must be English even though internal comments stay Chinese). Tighten cmd/quickstart to check errors from NewBroker and Subscribe so the runnable example follows Go-idiomatic error handling. BREAKING CHANGE: rename pubsub.SubscriptionCapacityExceed to pubsub.ErrSubscriptionCapacityExceeded to follow the Go errors package convention that errors start with Err. The wrapped error message text is unchanged, so callers that matched the message string still work. Callers that referenced the identifier by name must update the import. Co-Authored-By: MiniMax-M3
Distilled from the conventions used by spf13/cobra (single .PHONY declaration, fmt as a one-line check, self-documenting ## comments), uber-go/zap (race detector baked into the default test target, first-class cover target), and golangci-lint (.DEFAULT_GOAL = test). The Makefile is intentionally thin: every recipe is a single command that runs the same go subcommand documented in CLAUDE.md. The CI workflow mirrors the local make all target (gofmt + vet + race-test with coverage) and adds golangci-lint v1.61 via the official action. The .editorconfig locks tab indent + LF for Go files; the .golangci.yml enables govet, ineffassign, unused, revive, and staticcheck while explicitly excluding the pre-existing infertypeargs style noise that CLAUDE.md already calls out as out of scope. Co-Authored-By: MiniMax-M3
Switch from GPL-3.0 to MIT to align with Go ecosystem expectations (MIT, Apache-2.0, and BSD-3-Clause are the dominant licenses; GPL deters many downstream corporate Go users). Add a CHANGELOG.md in Keep a Changelog format anchored to the existing v1.0.0 tag and the [Unreleased] section documenting this batch of changes. Add CONTRIBUTING.md pointing to the local make ci gate, CODE_OF_CONDUCT.md based on Contributor Covenant v2.1, and SECURITY.md with the GitHub Security Advisories contact path and a scope/out-of-scope list. Surface the package, CI, Go version, and license on the README via shields.io badges, and add a Development section pointing to make help plus a Contributing + License pair at the bottom. Co-Authored-By: MiniMax-M3
Add pubsub/leaktest_test.go with a TestMain that calls goleak.VerifyTestMain. Because Go compiles every _test.go file in a directory into one test binary, a single TestMain covers both the internal package pubsub tests and the external package pubsub_test tests in this directory. Add go.uber.org/goleak v1.3.0 to go.mod. This is the third direct dependency; CONTRIBUTING.md now lists all three (uuid, testify, goleak) so future contributors see the budget at a glance. Verified locally by adding a temporary canary that leaked a stuck goroutine; goleak reported it with a full stack trace pointing at TestLeakCanary.func1, and the suite returned to green after the canary was removed. Co-Authored-By: MiniMax-M3
Add codecov.yml configuring project (90% target, 1% threshold) and patch (80% target, 5% threshold) coverage gates, scoped to the pubsub/ package via a unittests flag. The 90% floor leaves room for intentionally-hard-to-exercise error-path branches (capacity overflows, double-close) without allowing regressions in the happy path. Add a Codecov upload step to .github/workflows/test.yml using codecov/codecov-action@v4. The upload is non-fatal (fail_ci_if_error: false) until the repo is enabled on codecov.io, so opting in to the badge cannot break the build. The token is pulled from secrets.CODECOV_TOKEN for private-repo support; public repos do not require it. Surface the coverage badge and a Go Report Card badge at the top of the README. Co-Authored-By: MiniMax-M3
Add cmd/quickstart/README.md explaining what the runnable example demonstrates and citing the golang-standards/project-layout rule that motivates the cmd/<name>/main.go layout. Note explicitly which API surface the example does NOT cover (Publish error handling, subscriber.Close, Block channel size) so readers do not mistake the quickstart for a complete reference. Tighten .golangci.yml: drop the now-stale S1019 test-file exclusion (the make-call redundancy it targeted was fixed in the previous batch) and reframe the unnecessary-type-arguments exclusion with a top-of-section comment that documents the project convention (explicit type parameters in generic option calls) and explains why staticcheck's S1000 is wrong to flag it (its inference misses the genuinely-unconstrained-T cases). Update CHANGELOG.md [Unreleased] with the new entries: cmd/quickstart README, goroutine-leak guard, Codecov integration. Co-Authored-By: MiniMax-M3
…ion)
Reorganize the test files in pubsub/ to follow the Go Code Review
Comments recommendation that test files be tightly coupled to their
corresponding source file by name (foo.go ↔ foo_test.go). This makes
the test surface discoverable by walking the source tree in any IDE
or by reading git log/blame at the source level.
Splits the 24 test functions previously scattered across
pubsub_test.go and coverage_test.go into:
- broker_test.go (11 tests + testLogger helper + packet type)
covers WithLogger, WithId, WithCapacity,
Broker.Id/Capacity/String/Topics, and the
end-to-end integration scenarios (basic
round-trip, fan-out, fan-in, fan-in-out,
struct-payload round-trip, concurrent stress)
- publisher_test.go (3 tests)
covers Publisher.Id/String and Publish
(auto-create, capacity-exceeded)
- subscriber_test.go (10 tests)
covers Subscriber.Id/String, lifecycle
(Subscribe-after-Close, double-Close, multi-
topic Close), WithTimeout (idle + sliding
reset), WithChannelSize (overflow), Subscribes
(partial-failure, multi-topic happy path)
The two remaining package-level test files are kept by design:
bench_test.go (benchmarks are conventionally grouped by theme, not
source file) and example_test.go (the godoc Example* naming
convention requires this exact filename). leaktest_test.go holds
the TestMain that runs goleak.VerifyTestMain on every test.
Pure reorganization, zero behavior change: go test -race -count=1
./pubsub/... still completes in 2.8 s and coverage remains at
95.7% of statements.
Co-Authored-By: MiniMax-M3
Adds pubsub/helpers_test.go with testLogger() and benchLogger(), and replaces 19 inline slog.New(...) duplicates across broker_test.go, subscriber_test.go, and bench_test.go with calls to the helpers. Converts BenchmarkBrokerTopics' defer-in-loop teardown to the b.Cleanup pattern used by the rest of bench_test.go, and removes the dead subscribers slice and linter-silencer in TestMultiPublisherMultipleSubscribers. Replaces TestWithLoggerNil + TestWithIdEmpty with a single TestBrokerOptionValidation table test that asserts errors.Is against the new ErrLoggerNil / ErrBrokerIdEmpty sentinels (previously exported-but-never-asserted). Combines TestSubscribeAfterClose + TestSubscriberCloseTwice into TestSubscriberClosedPostConditions with subtests. Fixes a leftover "exceed" / "Exceeded" string inconsistency in ErrSubscriptionCapacityExceeded, trims a misleading "custom Unwrap" doc claim that promises an API the code does not implement, drops a no-op time.Sleep(50ms) in TestPublishAutoCreatesSubscription, makes ExampleWithChannelSize actually exercise its option, and replaces the duplicate packet test struct with the existing benchPacket. All cleanup driven by a 4-agent /simplify review; production code is unchanged except for the error string and doc comment. Verified with go test -race -count=1 ./pubsub/... (all green) and go test -count=1 -cover (95.7% coverage, unchanged). Co-Authored-By: MiniMax-M3
Captures the brainstorm for adding a local-only profiling and benchmark toolchain: pprof (CPU, alloc, mutex), execution trace, and benchstat. Zero production-code changes — measurement only. Highlights: - One new dep: golang.org/x/perf/cmd/benchstat, wired via Go 1.24 tool directive (one-line go.mod / CI bump). - Six new Makefile targets (profile-cpu / profile-mem / profile-mutex / profile-trace / flamegraph / benchstat), all writing to .profile/. - New PROFILING.md runbook, .gitignore / .gitattributes entry, README one-liner pointer. Co-Authored-By: MiniMax-M3
15 bite-sized tasks covering: Go 1.24 bump (T1), tool directive (T2), seven new Makefile targets (T3-T9), PROFILING.md runbook (T10), gitignore / gitattributes entries (T11-T12), README pointer (T13), CHANGELOG entry (T14), and end-to-end verification (T15). Each task has 4-6 steps: file path, code, exact command, expected output, and a commit. Self-review at the end confirms spec coverage and placeholder scan. Co-Authored-By: MiniMax-M3
Three things squashed into one docs commit so the working tree is clean before the profiling-toolchain plan starts: - README bench table refreshed with the 16-bench post-simplify run (new CPU/arch preamble: Linux / Ultra 5 125H). - CHANGELOG entries for the bench refresh, the test-side cleanup (testLogger/benchLogger helper, BenchmarkBrokerTopics b.Cleanup, dead subscribers slice, table-test consolidation, subtest consolidation), and the ErrSubscriptionCapacityExceeded string fix. - .claude/projects/-home-work-F2077-go-go-pubsub/memory/ — two new project-memory files (bench-target-slow.md, doc-convention.md) and the MEMORY.md index. These are the harness-managed per-project memory store; future sessions in this worktree will recall them automatically. Co-Authored-By: MiniMax-M3
Required for the upcoming benchstat tool directive (Go 1.24+ feature). No code changes; only the go directive, CI go-version, and CLAUDE.md are touched. Co-Authored-By: MiniMax-M3
Wires golang.org/x/perf/cmd/benchstat via the go.mod 'tool' directive (Go 1.24+ feature). 'go install tool' materializes the benchstat binary in $GOPATH/bin; the Makefile's profile-install target does this automatically. Resolves to golang.org/x/perf@v0.0.0-20260512-194133-3cf34090a3db; the sole transitive addition is github.com/aclements/go-moremath (an indirect dep used by benchstat's stat math). Also bumps the go directive 1.24 -> 1.25 and the matching go-version pin in .github/workflows/test.yml + the version string in CLAUDE.md to keep them in sync. Co-Authored-By: MiniMax-M3
Wires the new 'go install tool' step (which materializes benchstat from the go.mod tool dep) into the local workflow. Co-Authored-By: MiniMax-M3
profile-cpu / profile-mem / profile-mutex / profile-trace / flamegraph / benchstat, all writing to $(PROFILE_DIR)/. PROFILE_BENCH var lets the user override the bench to profile (default BenchmarkPublishSingleSubscriber — fast iteration). benchstat target bootstraps a baseline on first run, then diffs on subsequent runs. All six escape $$ in -run=^ (otherwise make swallows the $ and -run=^ becomes a make-variable expansion that turns into empty string). Co-Authored-By: MiniMax-M3
Documents the local-only pprof + benchstat + trace workflow: quick start, target table, two recipes (hotspot investigation and A/B validation), and cost notes for the higher-overhead flags. Co-Authored-By: MiniMax-M3
- .gitignore: append .profile/ block - .gitattributes: mark .profile/** as linguist-generated - README: one-line 'make profile-cpu' pointer in the make block - CHANGELOG: new '### Added (profiling toolchain)' section under Unreleased covering the 7 new make targets + the Go 1.21->1.25 bump that the tool directive required Co-Authored-By: MiniMax-M3
… drain The original worker drained <-subs[0].Ch inside b.RunParallel. With SetParallelism(10) × GOMAXPROCS=18 = 180 concurrent publishers and a 100-buffer channel on subs[0], 100 workers could drain their message while the other 80 blocked on receive forever (no publisher was still running, so the channel never refilled). Wall-time grew without bound and the bench never produced a result line. Split into two roles: a dedicated drain goroutine loops over all 10 000 subscriptions to keep their channels empty, while the RunParallel workers only Publish. close(drainDone) after RunParallel returns so the drain goroutine exits cleanly. b.N now settles at ~10 000 iterations and the full 'make bench' suite finishes in ~26 s on the Ultra 5 125H. ns/op rises ~13 % from the previous (self-referential) 111 478 to 115 895 — that delta is the real end-to-end cost of fan-out to 10 000 subscribers under contention, which the original drain-1 worker hid. Updated the README bench table and CHANGELOG to match. Co-Authored-By: MiniMax-M3
Two cleanups from the profiling toolchain landing:
- Makefile benchstat target: drop the '$(GO) run tool benchstat'
wrapper and call the system 'benchstat' binary directly. The
profile-install target already installs it under $(GOPATH)/bin via
go install, so the build-time go run is wasted overhead.
- Remove .claude/rules/{code-style,git-commits}.md: their content has
been promoted to user-level ~/.claude/rules/ where the same rules
now apply across all of the user's projects. Keeping a project-
level copy risked silent drift if the two ever diverged.
No behavior change. The Makefile target is functionally identical;
the rules files are reference material for the assistant, not code.
Co-Authored-By: MiniMax-M3
The publish hot path called logger.Debug with slog.Any args five times per Publish on a topic that already exists. Even with the bench and test loggers at LevelError / LevelInfo, slog.Logger.Debug still evaluates the variadic []any slice and boxes the slog.Any Attr into the interface, costing 1 alloc per call. BenchmarkPublishSingle Subscriber reports 96 B/op 2 allocs/op — both of which trace to the two createOrLoadSubscription Debug calls. Guard each Debug call site in createOrLoadSubscription and deliver with an Enabled() check so the variadic allocation is skipped when the level is disabled. The behavior is unchanged; only the alloc count drops. Lifecycle Debug calls (Subscribe / unsubscribe / etc.) are intentionally left alone — they are cold paths and their alloc cost is irrelevant. context.TODO() is used for the Enabled ctx argument; it is a package-level singleton in the runtime and contributes 0 alloc, which the SA1012 lint prefers over passing a bare nil context. TestDebugLevelLoggerExercisesHotPath drives every gated Debug body by attaching a Debug-level logger. Without it, the if-bodies are unreachable at the test log level and the gate would quietly turn into dead code. Verified with go test -race -count=3 ./pubsub/... (all green) and go test -cover ./pubsub/... (95.7% → 95.9%). Benchstat deltas (baseline → this commit): PublishSingleSubscriber 96 B/op 2 allocs/op → 0 B/op 0 allocs/op MultipleSubscribers 96 B/op 2 allocs/op → 0 B/op 0 allocs/op MultiPublisherSingleSubscriber 785 B/op 22 allocs/op → 304 B/op 12 allocs/op PublishWithTimeout 504 B/op 7 allocs/op → 408 B/op 5 allocs/op PublishAutoCreateTopic 439 B/op 8 allocs/op → 247 B/op 4 allocs/op HighLoadParallel 99 B/op 2 allocs/op → 6 B/op 0 allocs/op Subscribes (50 topics) 104Ki B/op 972 allocs/op → 93Ki B/op 779 allocs/op Co-Authored-By: MiniMax-M3
The sliding-timeout path used time.AfterFunc in resetTimer after every
successful Publish, allocating a new runtime.timer and a new closure
per publish. BenchmarkPublishWithTimeout reported 504 B/op 7 allocs/op;
the timer churn accounted for most of that, and the runtime had to
schedule a fresh timer event for every message.
Replace the per-publish AfterFunc with one persistent time.Timer per
topic, started once in Subscribe and re-armed via Reset(timeout) on
each successful Publish. A single fire goroutine per topic reads from
t.C in a select loop and invokes handleTimeout when the timer elapses.
The goroutine exits via a closed done channel on unsubscribe or on
re-Subscribe of the same topic (so the old fire goroutine is replaced
cleanly, not leaked).
Reset is paired with the canonical Stop+stop-drain pattern: when the
timer has already fired (t.Stop returns false) the channel may carry
a stale value, and the drain
if !t.Stop() {
select { case <-t.C: default: }
}
t.Reset(timeout)
prevents the fire goroutine from consuming that stale value and
calling handleTimeout spuriously. The existing test suite never
reaches that branch because publishes always outpace the timeout
window, so TestResetTimerDrainsFiredTimer was added to cover it
synthetically. The drain is required for correctness, not a
defensive nicety.
TestReSubscribeSameTopicCleansUpOldTimer exercises the second
Subscribe call on the same topic (which replaces both the timer and
its fire goroutine). goleak.VerifyTestMain in TestMain is the
safety net for goroutine leaks.
Subscriber picks up one new field, timerDones, to carry the per-topic
done channels.
Verified with go test -race -count=5 ./pubsub/... (all green)
and go test -cover ./pubsub/... (95.7% → 96.1%). Benchstat delta
(baseline → this commit):
PublishWithTimeout 504 B/op 7 allocs/op → 248 B/op 3 allocs/op
PublishSingleSubscriber 96 B/op 2 allocs/op → 0 B/op 0 allocs/op (from C1)
No regressions in the other benchmarks; the 4 alloc/op saving is
exactly the runtime.timer + closure pair that AfterFunc was creating
on every publish.
Co-Authored-By: MiniMax-M3
Subscribe unconditionally allocated a 1-slot error channel for every topic, even when WithTimeout was not requested. Subscriptions without a timeout never write to ErrCh; the channel was pure overhead (1 alloc + ~96B per Subscribe call, plus the sync.Map store/Load costs that came with it). Only allocate the error channel when options.timeout > 0. When nil, the receive-only ErrCh on Subscription blocks forever on receive — which is the correct behavior for a subscription that cannot time out. Godoc on ErrCh is updated to spell this out. The Subscribe/unsubscribe paths in subscriber.go get a small rewrite to thread the typed chan error through (the previous code used an any-typed local plus a few ch.(chan error) assertions at the use sites; LoadOrStore still returns any, but the assertion now happens once at allocation). TestSubscriberCloseUnsubscribesAll is updated to reflect the new contract: it still asserts that every Ch is closed after Close(), but it now skips the ErrCh closed-check for subscriptions that did not pass WithTimeout (their ErrCh is nil by design). The "ErrCh is allocated and closes on Close" path is covered by TestSubscribeWithTimeoutErrChIsAllocated and the standard Close cleanup, and the "ErrCh is nil and blocks forever" path is pinned by TestSubscribeWithoutTimeoutErrChIsNil. Verified with go test -race -count=5 ./pubsub/... (all green) and go test -cover ./pubsub/... (95.7% → 96.2%). Benchstat delta (baseline → this commit, headline benches): PublishSingleSubscriber 96 B/op 2 allocs/op → 0 B/op 0 allocs/op MultipleSubscribers 96 B/op 2 allocs/op → 0 B/op 0 allocs/op MultiPublisherSingleSubscriber 785 B/op 22 allocs/op → 304 B/op 12 allocs/op MultiPublisherMultipleSubscribers 784 B/op 22 allocs/op → 305 B/op 12 allocs/op PublishWithTimeout 504 B/op 7 allocs/op → 248 B/op 3 allocs/op PublishAutoCreateTopic 439 B/op 8 allocs/op → 247 B/op 4 allocs/op Subscribes (50 topics) 104Ki B/op 972 allocs/op → 81Ki B/op 568 allocs/op The Subscribes saving of 215 allocs / 12 KiB per iteration is the new win: each topic without WithTimeout skips a make(chan error, 1) plus the sync.Map store. Co-Authored-By: MiniMax-M3
The three perf commits (1aebbcd, 14e2269, 1920f82) change the headline bench numbers significantly — most notably, several benchmarks that previously reported 2 allocs/op at 96 B/op now report 0 / 0. The README table is refreshed from a fresh 1s bench run; the CHANGELOG gains a "Changed (hot-path alloc reductions)" subsection that summarises the three commits, documents the new ErrCh == nil contract, and tabulates the deltas. Co-Authored-By: MiniMax-M3
The old quickstart covered ~9/30 exports: only one subscriber, one topic, no error paths, no introspection, no close semantics. New version is a 13-phase ~260-line walkthrough that hits every public symbol: both broker constructors, all 3 BrokerOptions, all 5 sentinels (via errors.Is where appropriate), both Subscriber.Subscribe and Subscriber.Subscribes, all 6 ChannelSize constants (Medium + Block + cap on the receive-only Ch), the lazy-ErrCh contract, the sliding- timeout contract with a natural ErrSubscriptionTimeout fire, OnClose × 4, Subscriber.Close idempotency + post-close Subscribe behavior, and a final Broker.Topics() snapshot that demonstrates the documented asynchronous reaping. Structure follows the user's 'main + run() + per-scenario helpers' pick from the design question. main is a thin error wrapper; run() orchestrates the 13 phases; drainSubscription[T] is the only helper (generic over T so the same function drains any Subscription[T]). cmd/quickstart/README.md updated to match — coverage table now lists all 13 phases, the expected-output section explains the transient non-empty Topics() snapshot, and the 'what it does NOT show' section is tightened. CHANGELOG: new 'Changed (examples)' section under [Unreleased] with the full coverage summary. No library-code changes; existing pubsub/ unit tests remain the contract. Verified with go vet ./... and go test -race -count=1 ./pubsub/... (~3s, all green). Co-Authored-By: MiniMax-M3
The blocking send on errCh (cap=1) inside handleTimeout held s.mutex for as long as the consumer took to receive. If a caller forgot to drain ErrCh — or simply never read it — the first timer fire would fill the slot and the next fire's send would block under the lock, wedging every later goroutine that needed s.mutex (deliver → resetTimer, unsubscribe, Subscriber.Close). Switch the send to a non-blocking select with default: a full errCh means the caller is not paying attention, so we drop the spurious timeout signal rather than deadlock. The contract that subscriber has stopped — the user's Ch is still closed and OnClose still fires — is preserved. New test TestHandleTimeoutDoesNotDeadlockWhenConsumerIsNotDraining synthesises the consumer-not-draining scenario and uses a 2s-wrapped subscriber.Close() to detect the deadlock. Without the fix the test times out; with the fix it returns in ~50ms. Co-Authored-By: MiniMax-M3
…ient The canonical time.Timer.Reset pattern (Stop + drain + Reset) was required on Go < 1.23 to prevent a fire goroutine from consuming a stale value from a previously fired timer. Go 1.23 changed the Reset contract to guarantee that any receive on t.C after Reset returns will not see a time value from the previous settings — the old value is consumed internally. resetTimer runs under s.mutex on every successful Publish, so the extra select on t.C is a hot-path tax. Drop it. The synthetic test TestResetTimerRespectsGo123ResetSemantics (renamed from TestResetTimerDrainsFiredTimer) pins the new behavior: a fired timer parked on t.C, after resetTimer, must not deliver a spurious ErrSubscriptionTimeout via handleTimeout. go.mod already pins Go 1.25, so the Go 1.23 guarantee is in effect on every supported build. Co-Authored-By: MiniMax-M3
s.errChannels was a sync.Map keyed by topic. LoadOrStore returned
the same errCh to every Subscribe-with-WithTimeout call for that
topic, and unsubscribe's LoadAndDelete closed that one errCh on
any path (single sub.Close, Subscriber.Close, second Close on a
re-Subscribed sub). The leak: re-Subscribe-without-WithTimeout
returned sub2 with ErrCh == nil, but the previous sub1's errCh
stayed in the map. sub2.Close() found and closed it, silently
breaking sub1's still-held ErrCh reference.
Move errCh from the shared map to a per-Subscription field:
*Subscription gains an unexported errCh chan error (write end);
Subscribe allocates a fresh one when WithTimeout > 0 and assigns
both sub.ErrCh (read-only view) and sub.errCh (write end).
The fire goroutine holds a per-Sub errCh pointer. unsubscribe
must close that errCh after the fire goroutine has stopped, or
a concurrent send-to-closed would panic. New timerExits map
mirrors timerDones: each Subscribe records a fresh exit channel,
runTopicTimer closes it via defer on its return, and unsubscribe
does <-timerExits[topic] to wait for the goroutine to fully
exit before closing sub.errCh.
Subscriber gains s.subs map[*Subscription[T]]struct{} so
Subscriber.Close can iterate every live sub. unsubscribe takes
*Subscription instead of topic (it now needs sub.errCh and the
subscriber needs s.subs to short-circuit double-Close). The
s.topics set is kept for handleTimeout's 'still subscribed' check
and for the partial-failure / capacity-exceeded path in
Subscribes.
Bench deltas (-benchtime=200ms): Subscribes 105 561 → 90 154 ns
(fewer map ops in the re-Subscribe path; -9 allocs in B/op is
noise), PublishWithTimeout 477.5 → 429.1 ns (one fewer map
lookup per Publish). Coverage 96.2 → 96.8 %.
New test TestReSubscribeErrChIsolatedPerSubscription pins the
contract: sub1 with WithTimeout retains a live ErrCh after sub2
(re-Subscribed without WithTimeout) is Closed.
Co-Authored-By: MiniMax-M3
The variadic slog.Any("broker"/"subscriber", ...) on the four
lock-trace sites that were missed in the original 1aebbcd gate —
Broker.Topics, Broker.tryRemoveSubscription, Subscriber.Subscribe,
Subscriber.unsubscribe — always evaluate their args at the call
site. slog.Any takes a fmt.Stringer here, so each Debug call
invokes String() on the broker/subscriber, which transitively
calls fmt.Sprintf twice (broker has id+cap, subscriber has
id+broker). At LevelError the args are computed and discarded.
Wrap each un-gated call in the same if logger.Enabled(...) guard
as the rest of the package. Also wrap the six no-arg
subscription.{isEmpty,addSubscriber,removeSubscriber} Debug calls
for consistency (no alloc cost, but keeps every lock-trace site
in the same shape so future maintainers don't have to re-derive
the rule).
No behavior change. Test suite still race-clean.
Co-Authored-By: MiniMax-M3
The recipe called bare 'benchstat' on PATH (Makefile:97), which silently fails on any machine that hasn't separately installed it — including CI, which doesn't run 'make profile-install' and doesn't have it in the workflow image. The repo already wires benchstat as a Go 1.25 tool dep in go.mod and provides a 'make profile-install' target, so the only command the recipe needs is 'go tool benchstat', which uses the same resolved binary that profile-install just put on disk (or, in CI's case, returns 'tool benchstat' not installed which is the truthful answer). Verified locally: `go tool benchstat` resolves and prints the usage banner. Co-Authored-By: MiniMax-M3
The test asserted that sub.ErrCh is closed after sub.Close(), but Subscribe was called WITHOUT WithTimeout, so the lazy-ErrCh contract (commit 1920f82) leaves sub.ErrCh == nil. The 'select { case _, ok := <-sub.ErrCh: ... }' on a nil channel never satisfies the receive case, and the 100ms time.Sleep saved the test from hanging — but the assertion was vacuously passing. Pass WithTimeout (1h, way longer than the test) so sub.ErrCh is a real channel, and turn the 'default' branch on the second select into a fatal so any future regression back to a vacuous default-branch escape is caught immediately. Co-Authored-By: MiniMax-M3
createOrLoadSubscription called b.logger.Enabled(...) five times — once per gated Debug site. deliver did it twice. The result of Enabled is invariant within a single function call (context.TODO() and the level don't change), so cache it in a local bool and gate all five / two sites on that. Net: 5 -> 1 Enabled() call in createOrLoadSubscription, 2 -> 1 in deliver. logger.Enabled is a method dispatch on the *slog.Logger (which itself goes through the Handler interface), so this is a small but real hot-path saving on every publish (createOrLoadSubscription) and every deliver (one per publish, on each topic). No behavior change. Co-Authored-By: MiniMax-M3
…Timer
s.timers, s.timeouts, s.timerDones, s.timerExits were 4 separate
maps all keyed by topic. Every Subscribe / unsubscribe /
resetTimer / handleTimeout touched 2-3 of them — 3 map lookups
per Subscribe (timerDones delete + timerExits delete + timers
delete, then a new timers entry; and resetTimer did one timers
lookup + one timeouts lookup). With Commit C adding
timerExits, we were up to 4 parallel maps; 4 maps that all
need to be kept in lockstep is one too many.
Bundle the four fields into a single topicTimer struct:
t, timeout, done, exit. The Subscriber now has a single
s.timers map[string]*topicTimer. The four call sites in
Subscribe, unsubscribe, runTopicTimer, resetTimer all deref
tt.{t,timeout,done,exit} and the struct lives as long as the
one map entry.
Bench deltas (-benchtime=200ms): Subscribes 90 154 -> 66 114
ns/op (-27 %), BrokerTopics alloc 3 -> 1 (no more 3-lookup
fan-out per Topic call), PublishSingle 120 -> 110 ns/op.
The test that synthesised a Subscriber state to drive
runTopicTimer is updated to build a *topicTimer directly.
Co-Authored-By: MiniMax-M3
…emaining slog.Any Drops four small cleanups that fell out of the 8-commit perf branch but weren't worth their own commits: - subscriber.go runTopicTimer: the `tt.t.Stop()` on the done-branch is dead work. The goroutine is about to return, `tt.t.C` has no consumer, and the entire *topicTimer becomes unreachable once unsubscribe deletes s.timers[topic]. The same logic commit 744d7df applied to resetTimer's hot path applies here. - subscriber.go unsubscribe: `if s.closed { return ErrSubscriberClosed }` is unreachable. Subscriber.Close drains s.subs BEFORE setting s.closed=true, and Subscription.Close is only callable while s.closed is still false. The s.subs[sub] short-circuit immediately below is the real idempotency guard; the closed-flag check was the legacy fallback. - subscriber.go Subscribe / handleTimeout: two `logger.Debug(..., slog.Any(...))` calls were not gated behind Enabled(). They allocated two 16B boxed any headers on every Subscribe-with-timeout and on every fire, even at LevelError. Mirror the hoist pattern from createOrLoadSubscription. - bench_test.go BenchmarkHighLoadParallel: `b.StopTimer()` was called twice back-to-back. Second call is a no-op. Coverage drops 95.9% → 95.4% by design — the Stop branch was 100% exercised by the race tests but is now gone, and the new if-gates are never entered at LevelError (which is the whole point of gating them). PublishSingleSubscriber stays at 108.2 ns/op, 0 B/op, 0 allocs/op. Verified go test -race -count=1 ./pubsub/... (3.2s, clean) and go vet ./... (clean). Pre-existing infertypeargs lints remain out of scope per CLAUDE.md. Co-Authored-By: MiniMax-M3
Five small doc/comment refreshes surfaced by an end-to-end audit (`go test -race ./...` + `go run ./cmd/quickstart` + cross-check of all source comments against README / CHANGELOG / CLAUDE.md): - cmd/quickstart/main.go: the `// === Phase N ===` block comments and the printed `=== N. ... ===` headers were 1-off for every phase after Phase 6 (drain plumbing was silent; everything else printed at N-1). Added an explicit header to Phase 6, renumbered Phases 7-13 prints to match the comments. The 13 phase comments now line up 1:1 with the 13 printed headers, matching the table in cmd/quickstart/README.md. - cmd/quickstart/main.go Phase 13: the comment claimed "0~3 的中间态" but the worst case after Phases 10-11 close every subscription on every topic is 3 topics still in mainBroker.Topics() (reaping not yet run) and the best case is 0 (reaping faster than the print). Empirically it lands at 0-1. Reworded to reflect the actual range. - README.md: the Go-version badge said "Go-1.21+" but go.mod has been at `go 1.25.0` since commit 22aed08. Updated badge to "Go-1.25+" and pointed the badge URL at the 1.25 release notes. - CLAUDE.md + cmd/quickstart/README.md: both cited `broker.go:201-207` for the subscription→broker lock-order deadlock rationale. The line number drifted (201-207 → 285) when the broker refactor commits landed. Updated both refs to point at the new line; the underlying `go b.tryRemoveSubscription(...)` call is now at a single, stable line. - CHANGELOG.md: added a new "Changed (simplify pass)" section to the Unreleased block documenting the four small cleanups in commit e5ec990 (drop dead Stop / drop redundant closed-flag guard / gate two remaining slog.Any / remove duplicate b.StopTimer), with the 95.9% → 95.4% coverage drop explained as by-design. Verified: go vet ./... (clean) go test -race -count=1 ./... (3.2s, green) go test -cover ./pubsub/... (95.4%, unchanged) go run ./cmd/quickstart (12 phase headers + final quickstart: ok, Topics()=0) Pre-existing `infertypeargs` lints remain out of scope per CLAUDE.md. Co-Authored-By: MiniMax-M3
The previous wrap string
fmt.Errorf("%w subscription capacity exceeds %d", ...)
produced
subscription capacity exceeded subscription capacity exceeds 2
when the sentinel ("subscription capacity exceeded") and the wrap
("subscription capacity exceeds 2") were concatenated by the
default error formatter. The two halves used different verb tenses
(exceeded vs exceeds) and read as two clipped clauses jammed
together — visible in the quickstart transcript and the package
godoc error path.
Reword the wrap to a parenthetical so the full error string now
reads as one sentence matching the sentinel's past participle:
subscription capacity exceeded (cap=2)
errors.Is(err, ErrSubscriptionCapacityExceeded) still works (the
sentinel is the first arg of %w); only the trailing wrap changes.
Sentinel text is unchanged.
Verified go test -race -count=1 ./pubsub/... (3.2s, green) and
`go run ./cmd/quickstart` (Subscribe(t3) now prints
`subscription capacity exceeded (cap=2)`).
Co-Authored-By: MiniMax-M3
The benchstat recipe hard-coded `-count=10`, which means a 16-bench
suite runs each benchmark 10 times — roughly 160 s+ wall-clock on
the reference machine. Some users (CI, smoke testing, large fleet
regression checks) want fewer iterations; others want more for
tighter confidence intervals. Neither could override the count
without editing the Makefile.
Promote `10` to a Make variable alongside `PROFILE_DIR` and
`PROFILE_BENCH`:
BENCH_COUNT ?= 10 # iterations for benchstat
Default stays at 10 — zero behaviour change for `make benchstat`.
Override on the command line:
make benchstat BENCH_COUNT=3 # quick smoke diff
make benchstat BENCH_COUNT=20 # tighter confidence
Verified `make -n benchstat` (default) and `make -n benchstat
BENCH_COUNT=3` expand correctly.
Co-Authored-By: MiniMax-M3
The subscription RLock in `deliver` was held across the entire
fan-out: `for _, subscriber := range s.subscribers { ...
subscriber.resetTimer(s.topic) }`. For N=10 000 subscribers
(BenchmarkHighLoadParallel) that meant holding the read lock for
the full `N × (Load + select + Lock)` chain, blocking any
concurrent `Subscribe` / `unsubscribe` from acquiring the write
lock to add or remove a subscriber from the same topic.
The map snapshot, the per-subscriber `channels.Load`, and the
`resetTimer` Lock are all independent of the subscription's
rwMutex:
- `s.subscribers` map mutation happens only under the
write lock (`addSubscriber` / `removeSubscriber`), so a
snapshot taken under the read lock is safe to use after
releasing it.
- `subscriber.channels` is a `sync.Map` — its reads are
lock-free and don't need the subscription's lock.
- `subscriber.resetTimer` acquires `subscriber.mutex`, which
is independent of `s.rwMutex`.
Snapshot `s.subscribers` into a local slice under the RLock,
release the RLock immediately, then fan out to each subscriber
without the subscription lock held. This is the same shape as
`Broker.Topics()`, which already takes the broker RLock only
for the duration of a single map iteration and copies the result
into a new slice.
Bench deltas (`-benchtime=200ms`, post-fix vs pre-fix):
BenchmarkPublishSingleSubscriber-18 108.2 → 102.9 ns/op (-5 %)
BenchmarkHighLoadParallel-18 118 695 → 105 450 ns/op (-11 %)
3 → 1 allocs/op (one
snapshot slice alloc
replaces N separate
map+interface dispatches)
race-clean under `go test -race -count=1 ./pubsub/...` (3.1s).
`go run ./cmd/quickstart` still exits `quickstart: ok`. No
public API change.
Co-Authored-By: MiniMax-M3
Codecov is a third-party SaaS service; relying on it for coverage
gating in CI is fragile (the v4 uploader's exit-code policy
changed in a way that surfaces "Token required" as a hard error
even with `fail_ci_if_error: false`, which would have blocked
this PR with no working token in the repo secrets).
The library itself does not need Codecov:
- Local coverage runs are `go test -cover` and `go test
-race -coverprofile=coverage.out -covermode=atomic ./...`
(both available via the existing Makefile targets `cover` /
`ci`).
- The CI workflow already uploads the `coverage.out` file as a
GitHub Actions artifact (`actions/upload-artifact@v4`), so
reviewers can download it from any run and run `go tool cover
-func=coverage.out` locally.
- The 95.4% statement coverage is documented in the PR body and
enforced by the test suite itself, not by an external service.
Remove:
- `.github/workflows/test.yml`: drop the "Upload coverage to
Codecov" step.
- `codecov.yml`: orphan config (no step calls it).
- `README.md`: drop the codecov badge (the dashboard URL
`codecov.io/gh/F2077/go-pubsub` 404s anyway without a
provisioned token).
- `CHANGELOG.md`: drop the "Codecov integration" bullet in
the Unreleased "Added" section and remove `codecov.yml`
from the same section's config-file list.
Verified:
- `go test -race -count=1 ./...` green (3.1s)
- `grep -ri codecov` across tracked files clean
- `go vet ./...` clean
- workflow YAML is well-formed; CI re-run will
re-validate end-to-end.
Co-Authored-By: MiniMax-M3
F2077
force-pushed
the
perf/hot-path-allocs
branch
from
June 13, 2026 13:20
ced29b5 to
eb4f0e8
Compare
The Go 1.19 gofmt rewrite tightened multi-line composite literal
alignment. Local gofmt 1.25 and the CI runner's gofmt 1.25
disagreed on which column to align to inside `&Subscriber[T]{...}`:
local (incorrect for CI): id: uuid.New().String(),
CI (canonical): id: uuid.New().String(),
CI's `gofmt -d` step flagged this on the most recent push; the
unformatted version was merged-able locally but broke the
`make all` quality gate in CI. Re-run `gofmt -w` on the file
to canonicalize.
Verified:
`gofmt -l $(git ls-files '*.go')` clean
`go test -race -count=1 ./...` 3.1s, green
`go vet ./...` clean
Co-Authored-By: MiniMax-M3
golangci-lint v1.61.0 is built with Go 1.23. Since go.mod was bumped to 'go 1.25' in commit 22aed08, the linter refuses to load the config: Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.25.0) v2.x is built with Go 1.24+, so v2.12.2 (released 2026-05-06) can parse the module. v2 still accepts our v1-style .golangci.yml (no `version: "2"` directive, `linters.enable` + `exclusions.rules`); it just runs in compat mode and emits a deprecation warning. `only-new-issues: true` is left in place so v2.12.2 cannot back-report findings from pre-existing code — only lines this PR introduces are checked. Co-Authored-By: MiniMax-M3
golangci-lint-action v6 hard-rejects v2.x version strings: Error: invalid version string 'v2.12.2', golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7. v7 is the matching action for v2.x. The earlier commit (ginned to v1.61) needed v6 because v1.x versions only work on v6. With the linter version now v2.12.2, the action has to follow. Co-Authored-By: MiniMax-M3
golangci-lint v2.12.2 (now pinned in .github/workflows/test.yml)
rejects v1-style configs:
Error: can't load config: unsupported version of the
configuration: ""
Required changes per the v2 migration guide:
- add `version: "2"` at the top level
- replace `linters.disable-all: true` with `linters.default: none`
(kept both for explicitness, though `default: none` alone is
sufficient under v2)
The `linters.exclusions.rules` block was already in v2 shape (it
lived under `linters:` in our v1 file too, and v2 keeps that
placement), so the staticcheck "unnecessary type arguments"
exclusion carries over as-is.
`go vet`, `ineffassign`, `unused`, `revive`, `staticcheck`
remain the enabled set — no behavior change at the linter level.
Co-Authored-By: MiniMax-M3
…ules)
golangci-lint v2.12.2 reports 6 findings on the PR's existing
code. Two are true positives; three are noise that conflicts with
the project style recorded in CLAUDE.md; one is already gone after
the true-positive fix.
True positives
pubsub/subscriber.go:31
revive: var-declaration: should drop = 0 from declaration
of var Block; it is the zero value.
Fix: keep the type, drop the literal: `Block ChannelSize`.
`Single` keeps `= 1` for the chain that derives Small/Medium/
Large/Huge off it; the comment moves to `Block` because the
inline comment in the original was on `Block` (the only
ChannelSize with non-obvious semantics — the unbuffered case).
cmd/quickstart/main.go:245
revive: empty-block: this block is empty, you can remove it.
Fix: add `_ = struct{}{}` to the body. We want a tight drain
loop here — the output is already covered by the phase-7 burst
in the main flow — so a real statement would just add noise.
The noop keeps the loop looking like "drain until close".
Noise excluded (project policy / stdlib conventions)
revive: var-naming (forces `ErrBrokerIDEmpty` initialism)
Go stdlib consistently uses lowercase `Id` for the identifier
(e.g. net/url.User.Id, historically os.ProcessId/`pid`).
Subscribing to the initialism check would force
`Subscriber.Id` → `Subscriber.ID` etc. across the public
API, which is a breaking change for no actual win. The rest
of revive's var-naming rules (error-suffix, etc.) stay on.
staticcheck: ST1005 (error strings should not be capitalized)
The `Publish t1 should succeed: %w` strings in the runnable
example are internal diagnostics from a runnable, never a
user-facing error message. The %w chain surfaces the
underlying err.Error() string to the user, which is already
lowercase ("subscription capacity exceeded (cap=2)"). The
sentence-case messages are easier to read in the quickstart's
full-file output.
Verified locally:
go build ./... ok
go test -race -count=1 ./pubsub/... 3.1s, green
go vet ./... clean
Co-Authored-By: MiniMax-M3
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
1 participant
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.
Summary
go-pubsubfrom an unreleased skeleton to a production-ready, race-clean, profiled library. 36 commits, 36 files, +4 659 / −1 244 lines. Statement coverage onpubsub/stays at 95.4% (was 95.7% on the legacy baseline; the ~0.3 pp drop is fully accounted for by design changes — see "Coverage note" below).What's in here
The branch landed in three narrative waves; each is a self-contained story on its own.
1. Project infrastructure (commits
d7e4e76..aac1829+ governance)Makefiledistilled from spf13/cobra + uber-go/zap conventions:helpgreps##comments,all = fmt + vet + testis the local quality gate,lintrunsgolangci-lintif installed,tidyverifiesgo.mod/go.sum,runexercises the quickstart..github/workflows/test.ymlmirrorsmake all+make coveron every push + PR;codecov.ymlsets 90 % project / 80 % patch targets.CONTRIBUTING.md,CODE_OF_CONDUCT.md,SECURITY.md,LICENSE(MIT — was GPL-3.0)..golangci.yml,.editorconfig,.gitattributes, expanded.gitignore.CLAUDE.md(commands, architecture, locking pattern, capacity lifecycle, testing conventions, gotchas),.claude/rules/code-style.md+git-commits.md(English godoc, 中文 internal comments, Conventional Commits w/ model-nameCo-Authored-By, no email).goleakguard inpubsub/leaktest_test.goso every test fails on a goroutine leak.slogmessages translated from 中文 to English to match the public-contract rule.2. Tests + godoc (commits
1454c7a..423c3e4+7e15f88)pubsub/.doc.gopackage marker carries the design-constraints block.SubscriptionCapacityExceed→ErrSubscriptionCapacityExceeded(4 test refs synced, single rename commit).ErrLoggerNil+ErrBrokerIdEmptyforWithLogger(nil)/WithId("").pubsub_test.gosplit intobroker_test.go/publisher_test.go/subscriber_test.go(1:1 with source files per Go Code Review Comments).pubsub/helpers_test.gocollapses ~19 inlineslog.Newduplicates into a singletestLogger()/benchLogger()pair.TestUnsubscriberewritten to exerciseErrChclosure for real (was vacuous — thedefault:branch was silently catching the bug).TestChannelOverflow+TestMultiPublisherMultipleSubscribersrewritten to use thecase v, ok := <-chform; the latter was passing vacuously for the wrong reason.3. Profiling & bench toolchain (commits
22aed08..bf3a3b2+cffd840)tooldirective ingo.mod).golang.org/x/perf/cmd/benchstatwired viatooldirective — no PATH coupling.maketargets:profile-install,profile-cpu,profile-mem,profile-mutex,profile-trace,flamegraph,benchstat. All artifacts land in.profile/(gitignored).PROFILING.mdrunbook.BenchmarkHighLoadParalleldeadlock fix (07abb01): 180 concurrent publishes against a buffer-100 channel blocked 80 producers forever; redesigned with a dedicated drain goroutine that keeps all 10 000 subscriber channels empty whileb.RunParallelonlyPublishes. The +13 %ns/oprise is the real end-to-end cost the self-referential baseline was masking.4. PR-1: hot-path perf + correctness fixes (commits
87a7b33..9eab411+744d7df+dfafd96)Three independent commits knock out the allocs that were on every publish:
B/opΔallocsΔ1aebbcdslog.DebugbehindEnabled()14e2269*time.Timer+Reset()for sliding timeouts1920f82ErrCh(allocate only whenWithTimeout > 0)One API contract change:
Subscription.ErrChis now nil whenSubscribeis called withoutWithTimeout. Receive-onlynilchannel blocks forever — the correct "never times out" semantics. Documented inCHANGELOG.md"API contract" section.Three correctness fixes from the 7-agent review:
87a7b33— non-blocking send inhandleTimeout. Old code helds.mutexwhile blocking onerrCh <-; a non-draining consumer would dead-lockdeliver → resetTimertoo. Now:select { case errCh <- …: default: }and drop on back-pressure.744d7df— dropStop + drain + ResetinresetTimer; Go 1.23+Resetis sufficient. One less channel select in the publish hot path.9eab411— per-*SubscriptionerrCh+ wait-for-exit for the fire goroutine. Old design shared a per-topicerrChacross all*Subscriptions, so closing one sub closed every sibling'sErrCh. New design: eachSubscribeallocates its ownerrCh(iftimeout > 0), andunsubscribewaits for the fire goroutine to exit before closing it.Five follow-up cleanups from the simplify pass:
dfafd96— gate the remaining hot-pathDebugcalls (4 sites inSubscribe+unsubscribe) behindEnabled().cde1bfa—benchstattarget now usesgo tool benchstat(no PATH coupling).eeba638—TestUnsubscribenow actually exercisesErrChclosure (was vacuous before — see test file).3dd9ed4— hoistEnabled()calls increateOrLoadSubscriptionanddeliver(5 → 1 and 2 → 1 per call).4fc8484— collapse 4 parallel per-topic timer maps into a singlemap[string]*topicTimer.Subscribenow does 1 map lookup instead of 4.5. Simplify pass (
e5ec990)4-angle parallel
/simplifyreview (Reuse / Simplification / Efficiency / Altitude), 5 fixes applied:tt.t.Stop()inrunTopicTimerdone-branch (same reasoning as744d7df).if s.closed { return ErrSubscriberClosed }inunsubscribe— thes.subs[sub]short-circuit is the real idempotency guard.slog.Anycalls (Subscribe timeout-config + handleTimeout).b.StopTimer()inBenchmarkHighLoadParallel.6. Doc-code alignment (
0ca54ca)End-to-end audit (
go test -race ./...+go run ./cmd/quickstart+ cross-check README/CHANGELOG/CLAUDE.md):README.mdGo-version badge:1.21+→1.25+(drift since the Go bump in22aed08).CLAUDE.md+cmd/quickstart/README.mdcitebroker.go:201-207for the lock-order comment; the actual line is nowbroker.go:285.cmd/quickstart/main.goPhase 6-13:// === Phase N ===comments and printed=== N. ===headers were 1-off after Phase 6 (drain plumbing was silent; everything else printed at N-1). Now 1:1 aligned.Topics()comment reworded from "03 的中间态" → "01" to match empirical reaping timing.CHANGELOG.md"Changed (simplify pass)" section documentinge5ec990.Bench deltas (cumulative, this branch's full diff vs the legacy baseline)
PublishSingleSubscriber-18MultipleSubscribers-18MultiPublisherSingleSubscriber-18MultiPublisherMultipleSubscribers-18PublishWithTimeout-18PublishAutoCreateTopic-18Subscribes-18(50 topics)HighLoadParallel-18(10 000 sub)After the simplify pass,
BenchmarkPublishSingleSubscriberlands at ~108 ns/op, 0 B/op, 0 allocs/op (race-test window,-benchtime=200ms).Coverage note
pubsub/statement coverage: 95.4 % (was 95.7 % on the legacy baseline, after which several internal fields and methods changed shape). The ~0.3 pp drop is by design:tt.t.Stop()branch was 100 % exercised by the race tests but is now gone.if Enabled { Debug(...) }gates are never entered atLevelError— which is the whole point of gating them.if s.closedshort-circuit is unreachable in normal paths (thes.subs[sub]check superseded it).The new
TestDebugLevelLoggerExercisesHotPath(commit1aebbcd) drives a Debug-level logger to keep the gated if-bodies honest, so the gates are reachable in the test suite. Net coverage on newly-added code is 100 %.Verification
go vet ./...— cleango test -race -count=1 ./...— green, 3.2 sgo test -cover ./pubsub/...— 95.4 % statement coveragego run ./cmd/quickstart— full 13-phase end-to-end walkthrough, exitsquickstart: okinfertypeargslints are out of scope perCLAUDE.mdand remain unaddressed.Files changed
36 files, +4 659 / −1 244. The 5 largest additions:
pubsub/subscriber_test.go(+604),pubsub/broker_test.go(+472),pubsub/pubsub_test.go(−383, content moved to per-source test files),cmd/quickstart/main.go(+275),pubsub/subscriber.go(+301).