Avoid allocating tintError twice#82
Merged
lmittmann merged 2 commits intolmittmann:mainfrom Dec 16, 2024
Merged
Conversation
Passing the type-asserted tintError value as an error causes a second allocation. Since the helper method is only for tintError values, change the signature to specify the concrete type to elide the second allocation.
Owner
|
I can't verify your claim in the allocs of the benchmark. Can you provide some instructions to verify this? |
Contributor
Author
|
I discovered this because it showed up in VSCode Go extension's "Toggle GC details" when I was looking for the extra allocations that had led to me opening #83. 
Here's a simple benchmark: func BenchmarkError(b *testing.B) {
logger := slog.New(tint.NewHandler(io.Discard, nil))
err := slog.Any("error", errTest)
terr := tint.Err(errTest)
b.Run("error", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
logger.LogAttrs(context.TODO(), slog.LevelError, testMessage, err)
}
})
b.Run("tint.Err", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
logger.LogAttrs(context.TODO(), slog.LevelError, testMessage, terr)
}
})
}Before the change: goos: darwin
goarch: arm64
pkg: github.com/lmittmann/tint
cpu: Apple M3
BenchmarkError/error-8 3042105 364.5 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3246169 372.4 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3190713 374.9 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3185554 377.8 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3129958 379.8 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3157929 380.9 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3139827 381.1 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3116568 383.8 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3161204 384.8 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3121477 386.3 ns/op 4 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3589369 333.8 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3555742 334.5 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3583209 332.6 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3605314 334.1 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3577548 332.8 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3609236 332.3 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3585714 335.8 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3538658 333.1 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3552102 333.6 ns/op 16 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3581828 334.2 ns/op 16 B/op 1 allocs/op
PASS
ok github.com/lmittmann/tint 32.174sAfter the change: goos: darwin
goarch: arm64
pkg: github.com/lmittmann/tint
cpu: Apple M3
BenchmarkError/error-8 3219517 372.6 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3178566 379.3 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3159981 380.4 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3114860 381.9 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3119835 382.8 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3116502 381.6 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3124263 381.1 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3126104 381.7 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3125305 382.7 ns/op 4 B/op 1 allocs/op
BenchmarkError/error-8 3137502 382.9 ns/op 4 B/op 1 allocs/op
BenchmarkError/tint.Err-8 3787142 318.7 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3745261 315.6 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3771666 317.8 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3740589 314.6 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3848024 323.0 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3727220 314.9 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3810910 314.7 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3814754 315.8 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3793502 317.7 ns/op 0 B/op 0 allocs/op
BenchmarkError/tint.Err-8 3717480 312.8 ns/op 0 B/op 0 allocs/op
PASS
ok github.com/lmittmann/tint 31.755sBenchstat: goos: darwin
goarch: arm64
pkg: github.com/lmittmann/tint
cpu: Apple M3
│ before.txt │ after.txt │
│ sec/op │ sec/op vs base │
Error/error-8 380.3n ± 2% 381.7n ± 1% ~ (p=0.493 n=10)
Error/tint.Err-8 333.7n ± 0% 315.7n ± 1% -5.39% (p=0.000 n=10)
geomean 356.3n 347.1n -2.57%
│ before.txt │ after.txt │
│ B/op │ B/op vs base │
Error/error-8 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹
Error/tint.Err-8 16.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10)
geomean 8.000 ? ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean
│ before.txt │ after.txt │
│ allocs/op │ allocs/op vs base │
Error/error-8 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹
Error/tint.Err-8 1.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10)
geomean 1.000 ? ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean |
Contributor
Author
|
I wrote this simple benchmark because the full Maybe we should simply replace |
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
2 participants
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.
Passing the type-asserted
tintErrorvalue as an error causes a second allocation.Since the helper method is only for
tintErrorvalues, change the signature to specify the concrete type to elide the second allocation.