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

Only convert to any on KindAny values #83

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

database64128
Copy link
Contributor

Indiscriminately calling slog.Value.Any() on the value causes extra allocations for non-any values. Benchmark shows that for each string attribute, an extra 16-byte allocation is incurred.

This commit adds a check so Any() is only called on values of KindAny.

Indiscriminately calling slog.Value.Any() on the value causes extra allocations for non-any values. Benchmark shows that for each string attribute, an extra 16-byte allocation is incurred.

This commit adds a check so Any() is only called on values of KindAny.
@lmittmann lmittmann merged commit e3abebc into lmittmann:main Dec 10, 2024
2 checks passed
@lmittmann
Copy link
Owner

Thanks for the PR!

                                      │     old      │                new                  │
                                      │    sec/op    │   sec/op     vs base                │
LogAttrs/h=tint/5_args-8                 1.610µ ± 1%   1.458µ ± 1%   -9.44% (p=0.000 n=10)
LogAttrs/h=tint/5_args_custom_level-8    1.613µ ± 0%   1.479µ ± 1%   -8.31% (p=0.000 n=10)
LogAttrs/h=tint/10_args-8                2.872µ ± 0%   2.605µ ± 1%   -9.31% (p=0.000 n=10)
LogAttrs/h=tint/40_args-8               10.164µ ± 1%   9.087µ ± 1%  -10.60% (p=0.000 n=10)
geomean                                  2.950µ        2.673µ        -9.42%

                                      │     old      │                 new                  │
                                      │     B/op     │     B/op      vs base                │
LogAttrs/h=tint/5_args-8                  96.00 ± 0%     36.00 ± 0%  -62.50% (p=0.000 n=10)
LogAttrs/h=tint/5_args_custom_level-8     96.00 ± 0%     36.00 ± 0%  -62.50% (p=0.000 n=10)
LogAttrs/h=tint/10_args-8                 400.0 ± 0%     280.0 ± 0%  -30.00% (p=0.000 n=10)
LogAttrs/h=tint/40_args-8               2.126Ki ± 0%   1.657Ki ± 0%  -22.05% (p=0.000 n=10)
geomean                                   299.3          157.5       -47.37%

                                      │    old      │                new                 │
                                      │  allocs/op  │ allocs/op   vs base                │
LogAttrs/h=tint/5_args-8                 6.000 ± 0%   2.000 ± 0%  -66.67% (p=0.000 n=10)
LogAttrs/h=tint/5_args_custom_level-8    6.000 ± 0%   2.000 ± 0%  -66.67% (p=0.000 n=10)
LogAttrs/h=tint/10_args-8               13.000 ± 0%   5.000 ± 0%  -61.54% (p=0.000 n=10)
LogAttrs/h=tint/40_args-8                49.00 ± 0%   17.00 ± 0%  -65.31% (p=0.000 n=10)
geomean                                  12.31        4.294       -65.11%

@database64128 database64128 deleted the avoid-alloc branch December 16, 2024 18:24
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