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

Add benchmarks for chained Withs #1326

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

jquirke
Copy link
Contributor

@jquirke jquirke commented Aug 18, 2023

Add benchmarks to show the performance of chaining child loggers via 'With' and whether the logger is used or not used.

This is intended to establish a baseline for a proposed optimisation in PR #1319.

@jquirke jquirke changed the title Add Benchmarks for chained withs Add benchmarks for chained Withs Aug 18, 2023
Copy link
Collaborator

@abhinav abhinav left a comment

Choose a reason for hiding this comment

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

In favor of this change. Just leaving a minor suggestion for if/when we want to expand this benchmark.

logger_bench_test.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #1326 (8f81672) into master (4de7706) will decrease coverage by 0.15%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1326      +/-   ##
==========================================
- Coverage   97.77%   97.62%   -0.15%     
==========================================
  Files          51       51              
  Lines        3366     3366              
==========================================
- Hits         3291     3286       -5     
- Misses         65       69       +4     
- Partials       10       11       +1     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jquirke jquirke force-pushed the benchmark-chained-loggers branch 4 times, most recently from 3864e3c to 6be7c84 Compare August 18, 2023 22:09
Add benchmarks to show the performance of chaining child loggers via 'With' and
whether the logger is used or not used.

This is intended to establish a baseline for a proposed optimisation in PR
uber-go#1319, in particular, we would like to see
if we can avoid the expense of With() if a logger is never used.

Currently, the performance is similar irrespective of whether the logger is used
or not:

```
 % go test -bench=Benchmark5Withs
goos: darwin
goarch: arm64
pkg: go.uber.org/zap
Benchmark5WithsUsed-10       	  512342	      2276 ns/op
Benchmark5WithsNotUsed-10    	  570768	      2201 ns/op
```
@abhinav abhinav merged commit c50301e into uber-go:master Aug 18, 2023
5 checks passed
@rabbbit
Copy link
Contributor

rabbbit commented Aug 19, 2023

I'm late, but thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants