Reduce metric emission and construction overhead#132
Merged
Conversation
Optimizes the spectator-go v2 hot paths that regressed relative to the pre-spectatord aggregating registry (v0.2.0), focused on the dominant `registry.Counter(name, tags).Increment()`-per-call pattern. Zero-allocation emission via typed writer methods: - Add WriteLine/WriteInt/WriteUint/WriteFloat to the writer.Writer interface and all implementations. Buffered writers append the line prefix and value directly into their backing storage (LowLatencyBuffer into its shard chunks, LineBuffer into its builder), and numeric values are formatted with strconv.Append* into a stack buffer, so emission no longer allocates the intermediate "prefix + value" string. Non-buffered writers concatenate as a fallback, no worse than the previous Write path. LowLatencyBuffer shards on the prefix alone (the meter id lies between the first and last ':', both in the prefix), equivalent to full-line sharding. Cheaper metric-id sanitization: - writeSanitized now byte-scans for validity and, when the input is already clean (the common case), writes it in one shot instead of a per-rune UTF-8 decode + re-encode. Byte-identical output; only clean-vs-dirty routing added. Direct meter construction from (name, tags): - Registry (name, tags) methods build the line prefix directly via buildLinePrefix, without allocating an *Id or copying the tags map. The meter stores only its line prefix; MeterId() reconstructs an Id on demand for the rarely-used introspection path. extraCommonTags are merged into the prefix (common tags win on key collision), so meters built this way carry the same infrastructure tags as the WithId path. Measured, cached meter -> LowLatencyBuffer: Counter.Increment: 41 ns, 96 B, 1 alloc -> 24 ns, 0 B, 0 allocs Counter.Add: 59 ns, 119 B, 2 allocs -> 33 ns, 0 B, 0 allocs Gauge.Set/Timer.Record/DistSummary.Record likewise reach 0 allocs. Measured, construct-per-call (registry.Counter(name,tags).Increment()): v2 before: 544 ns, 5 allocs v2 now: 175 ns, 2 allocs (via registry) v0.2.0: 384 ns, 6 allocs so v2 is now faster than the version the pattern regressed from, with no registry meter cache. Notes: - Adds methods to the exported writer.Writer interface: a breaking change for any external Writer implementation (there is no supported way to inject a custom Writer, so in-tree impact is nil). Worth a release-note callout. - MeterId() on meters built via the (name, tags) path returns an Id whose name/tags are the sanitized, prefix-reconstructed forms (lossy for unclean input) rather than the caller's originals. Includes end-to-end emit benchmarks and correctness tests: WriteLine/typed output is byte-identical to Write(prefix+value); direct-vs-WithId identity matches; and the (name, tags) path preserves common tags (with merge/override).
Contributor
Author
Benchmark impact (PR #129
|
| Benchmark | before | after | allocs/op |
|---|---|---|---|
Counter_Increment |
25 ns | 2.1 ns | 1 → 0 |
Counter_Add |
50 ns | 2.1 ns | 1 → 0 |
Gauge_Set |
61 ns | 2.0 ns | 1 → 0 |
Gauge_SetInt |
31 ns | 2.0 ns | 1 → 0 |
Timer_Record |
70 ns | 2.2 ns | 1 → 0 |
DistributionSummary_Record |
48 ns | 2.1 ns | 1 → 0 |
The per-emit allocation is eliminated. The ~2 ns figure is meter-side cost only
(NoopWriter discards); a real buffer still formats the value, but does so
allocation-free — the end-to-end LowLatencyBuffer path is 24 ns / 0 allocs for
Increment.
Id creation & sanitization (writeSanitized fast-path)
| Benchmark | before | after | Δ |
|---|---|---|---|
NewId |
369 ns | 250 ns | −32% |
NewId_TagScaling/tags-0…10 |
— | — | −29% … −38% |
WithTag |
423 ns | 261 ns | −38% |
WithTags |
487 ns | 338 ns | −31% |
ToSpectatorIdBuilder |
850 ns | 610 ns | −28% |
ToSpectatorId_Current/5Tags |
627 ns | 226 ns | −64% |
ToSpectatorId_Current/InvalidChars |
393 ns | 394 ns | 0% |
Allocations unchanged (pure CPU win). InvalidChars is flat because unclean
input still takes the per-rune slow path, as intended.
Construct-per-call (*_VaryingTags: NewId + NewX(id) + emit each iteration)
| Benchmark | before | after | allocs/op |
|---|---|---|---|
Counter_VaryingTags |
538 ns | 355 ns (−34%) | 8 → 7 |
Gauge_VaryingTags |
563 ns | 348 ns (−38%) | 8 → 7 |
Timer_VaryingTags |
577 ns | 357 ns (−38%) | 8 → 7 |
DistributionSummary_VaryingTags |
871 ns | 365 ns (−58%) | 9 → 7 |
Faster sanitization plus one fewer allocation (the removed emit concat). Note
this benchmark uses the WithId constructor, so it still builds an Id; the
registry.Counter(name, tags) path is faster still (5 → 2 allocs, ~175 ns).
Notes
BenchmarkToSpectatorIdandToSpectatorId_V1*are reference baselines
(inline copies of the old implementations) unaffected by this change; their
±5–15% deltas are measurement noise.- Absolute ns are from a laptop under load — re-run on target hardware for
reporting.
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.
Optimizes the spectator-go v2 hot paths that regressed relative to the pre-spectatord aggregating registry (v0.2.0), focused on the dominant
registry.Counter(name, tags).Increment()-per-call pattern.Zero-allocation emission via typed writer methods:
Cheaper metric-id sanitization:
Direct meter construction from (name, tags):
Notes:
Includes end-to-end emit benchmarks and correctness tests: WriteLine/typed output is byte-identical to Write(prefix+value); direct-vs-WithId identity matches; and the (name, tags) path preserves common tags (with merge/override).