Skip to content

internal/tags: use slices.SortFunc instead of sort.Sort - #182

Closed
th0114nd wants to merge 1 commit into
masterfrom
zero-alloc-tags-01-sorting
Closed

internal/tags: use slices.SortFunc instead of sort.Sort#182
th0114nd wants to merge 1 commit into
masterfrom
zero-alloc-tags-01-sorting

Conversation

@th0114nd

@th0114nd th0114nd commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

sort.Sort(t) requires boxing the TagSet into a sort.Interface, which
allocates on every call since TagSet doesn't fit in a single interface
word. slices.SortFunc is generic and takes the comparison function
directly, avoiding that allocation entirely.

Bumps the module's minimum Go version to 1.21, when the stdlib slices
package was introduced.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com


Stack created with GitHub Stacks CLIGive Feedback 💬

sort.Sort(t) requires boxing the TagSet into a sort.Interface, which
allocates on every call since TagSet doesn't fit in a single interface
word. slices.SortFunc is generic and takes the comparison function
directly, avoiding that allocation entirely.

Bumps the module's minimum Go version to 1.21, when the stdlib slices
package was introduced.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment thread go.mod
module github.com/lyft/gostats

go 1.18
go 1.21

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is breaking in that it relies on importers to upgrade to 1.21. The alternative is to add a dependency on golang.org/x/exp/slices.

All OSS dependents on gostats are forks of envoyproxy/ratelimit, and envoyproxy itself is at go1.26: https://github.com/envoyproxy/ratelimit/

https://pkg.go.dev/github.com/lyft/gostats?tab=importedby

@th0114nd

th0114nd commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Benchmark: allocation impact

Ran SerializeTags before/after this change (master @ 5f2727b vs this branch), -race=false to avoid race-detector overhead skewing results:

tags before (allocs, B/op, ns/op) after (allocs, B/op, ns/op) delta
0 0, 0B, 3.75ns 0, 0B, 3.76ns no change (untouched fast path)
1 1, 24B, 63.60ns 1, 24B, 63.22ns no change (untouched fast path)
2 1, 48B, 89.17ns 1, 48B, 89.12ns no change (untouched fast path)
4 1, 64B, 138.6ns 1, 64B, 138.2ns no change (untouched fast path)
8 3, 392B, 256.1ns 2, 368B, 212.5ns -1 alloc, ~17% faster
16 3, 760B, 665.9ns 2, 736B, 616.6ns -1 alloc, ~7% faster

The 1-4 tag cases are unaffected as expected — they use hand-unrolled fast paths this change doesn't touch. The >4 tag default branch drops from 3 allocations to 2: pairs := make(TagSet, 0, numValid) (unchanged), sort.Sort(pairs)'s interface-boxing of the TagSet into a sort.Interface (removed — this is the allocation slices.SortFunc avoids), and the final output buffer (unchanged).

Methodology: go test -race=false -bench=BenchmarkSerializeTags -benchmem, GOFLAGS= to override the ambient -race default in this environment; each row averages b.N iterations via the standard testing.B loop.

@th0114nd th0114nd closed this Aug 4, 2026
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