Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid allocating tintError twice #82

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

database64128
Copy link
Contributor

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.

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.
@lmittmann
Copy link
Owner

I can't verify your claim in the allocs of the benchmark. Can you provide some instructions to verify this?

@database64128
Copy link
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.

image

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.174s

After 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.755s

Benchstat:

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

@database64128
Copy link
Contributor Author

I wrote this simple benchmark because the full BenchmarkLogAttrs seemed a bit much for my laptop, and I'm currently not around my desktop. You should be able to observe the same difference if you add tint.Err to BenchmarkLogAttrs.

Maybe we should simply replace slog.Any("error", errTest) with tint.Err(errTest) in BenchmarkLogAttrs. What do you think?

@lmittmann lmittmann merged commit bd5634c into lmittmann:main Dec 16, 2024
2 checks passed
@database64128 database64128 deleted the optimize-err branch December 16, 2024 18:26
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.

2 participants