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
zapcore: add stopped for Stop method called exactly once #966
Conversation
name old time/op new time/op delta BufferedWriteSyncer/write_file_with_buffer-20 137ns ± 1% 139ns ± 2% ~ (p=0.082 n=5+6) MultiWriteSyncer/2_discarder-20 8.05ns ± 6% 7.93ns ± 2% ~ (p=0.662 n=6+5) MultiWriteSyncer/4_discarder-20 9.15ns ± 4% 9.14ns ± 4% ~ (p=1.000 n=5+5) MultiWriteSyncer/4_discarder_with_buffer-20 131ns ± 2% 130ns ± 0% ~ (p=0.370 n=6+6) WriteSyncer/write_file_with_no_buffer-20 4.36µs ± 1% 4.24µs ± 2% -2.96% (p=0.004 n=5+6) ZapConsole-20 787ns ± 1% 800ns ± 2% ~ (p=0.052 n=5+6) JSONLogMarshalerFunc-20 738ns ± 1% 737ns ± 0% ~ (p=0.970 n=6+6) ZapJSON-20 586ns ± 4% 582ns ± 3% ~ (p=0.818 n=6+6) StandardJSON-20 991ns ± 9% 977ns ± 2% ~ (p=0.892 n=6+5) Sampler_Check/7_keys-20 8.58ns ± 3% 8.49ns ± 2% ~ (p=0.485 n=6+6) Sampler_Check/50_keys-20 8.18ns ± 1% 8.20ns ± 1% ~ (p=0.690 n=5+5) Sampler_Check/100_keys-20 8.02ns ± 4% 8.02ns ± 2% ~ (p=0.589 n=6+6) Sampler_CheckWithHook/7_keys-20 23.4ns ± 2% 23.2ns ± 2% ~ (p=0.370 n=6+6) Sampler_CheckWithHook/50_keys-20 24.3ns ± 3% 24.8ns ± 7% ~ (p=0.732 n=6+6) Sampler_CheckWithHook/100_keys-20 24.1ns ± 2% 24.2ns ± 1% ~ (p=0.662 n=5+6) TeeCheck-20 118ns ± 1% 120ns ± 5% ~ (p=1.000 n=4+6) [Geo mean] 77.7ns 77.6ns -0.08% name old alloc/op new alloc/op delta BufferedWriteSyncer/write_file_with_buffer-20 16.0B ± 0% 16.0B ± 0% ~ (all equal) MultiWriteSyncer/2_discarder-20 16.0B ± 0% 16.0B ± 0% ~ (all equal) MultiWriteSyncer/4_discarder-20 16.0B ± 0% 16.0B ± 0% ~ (all equal) MultiWriteSyncer/4_discarder_with_buffer-20 16.0B ± 0% 16.0B ± 0% ~ (all equal) WriteSyncer/write_file_with_no_buffer-20 16.0B ± 0% 16.0B ± 0% ~ (all equal) ZapConsole-20 1.36kB ± 0% 1.36kB ± 0% ~ (all equal) JSONLogMarshalerFunc-20 1.31kB ± 0% 1.31kB ± 0% ~ (all equal) ZapJSON-20 1.25kB ± 0% 1.25kB ± 0% ~ (all equal) StandardJSON-20 1.45kB ± 0% 1.45kB ± 0% ~ (all equal) Sampler_Check/7_keys-20 0.00B 0.00B ~ (all equal) Sampler_Check/50_keys-20 0.00B 0.00B ~ (all equal) Sampler_Check/100_keys-20 0.00B 0.00B ~ (all equal) Sampler_CheckWithHook/7_keys-20 0.00B 0.00B ~ (all equal) Sampler_CheckWithHook/50_keys-20 0.00B 0.00B ~ (all equal) Sampler_CheckWithHook/100_keys-20 0.00B 0.00B ~ (all equal) TeeCheck-20 64.0B ± 0% 64.0B ± 0% ~ (all equal) [Geo mean] 108B 108B +0.00% name old allocs/op new allocs/op delta BufferedWriteSyncer/write_file_with_buffer-20 1.00 ± 0% 1.00 ± 0% ~ (all equal) MultiWriteSyncer/2_discarder-20 1.00 ± 0% 1.00 ± 0% ~ (all equal) MultiWriteSyncer/4_discarder-20 1.00 ± 0% 1.00 ± 0% ~ (all equal) MultiWriteSyncer/4_discarder_with_buffer-20 1.00 ± 0% 1.00 ± 0% ~ (all equal) WriteSyncer/write_file_with_no_buffer-20 1.00 ± 0% 1.00 ± 0% ~ (all equal) ZapConsole-20 7.00 ± 0% 7.00 ± 0% ~ (all equal) JSONLogMarshalerFunc-20 5.00 ± 0% 5.00 ± 0% ~ (all equal) ZapJSON-20 3.00 ± 0% 3.00 ± 0% ~ (all equal) StandardJSON-20 27.0 ± 0% 27.0 ± 0% ~ (all equal) Sampler_Check/7_keys-20 0.00 0.00 ~ (all equal) Sampler_Check/50_keys-20 0.00 0.00 ~ (all equal) Sampler_Check/100_keys-20 0.00 0.00 ~ (all equal) Sampler_CheckWithHook/7_keys-20 0.00 0.00 ~ (all equal) Sampler_CheckWithHook/50_keys-20 0.00 0.00 ~ (all equal) Sampler_CheckWithHook/100_keys-20 0.00 0.00 ~ (all equal) TeeCheck-20 1.00 ± 0% 1.00 ± 0% ~ (all equal) [Geo mean] 2.21 2.21 +0.00% Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
3306b8c
to
69b22fd
Compare
Codecov Report
@@ Coverage Diff @@
## master #966 +/- ##
=======================================
Coverage 98.16% 98.17%
=======================================
Files 45 45
Lines 2020 2027 +7
=======================================
+ Hits 1983 1990 +7
Misses 29 29
Partials 8 8
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm supportive of allowing Stop
to be called twice, though I wonder if we can do it with a simpler implementation, taking advantage of the existing mutex.
This patch changes behaviour of Stop
slightly. Previously, if Stop
was called before initialization, it was a no-op, and calls after it was initialized would actually stop it. Now, the first Stop
will result in the sync.Once
being completed, and further calls won't have any impact (nothing is stopped). Checking for a stopped bool
under the mutex may be a simpler alternative that avoids the behaviour change.
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@prashantv changed to stopped bool and add local value for avoid call mutex twice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
@prashantv Thanks! |
Thanks for the contribution @zchee! |
Add
stopOnce
field toBufferedWriteSyncer
for Stop method called exactly once.benchmark results mostly no-op: