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.

Sign up for GitHub

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

Jump to bottom

make Discard logger equal to null logger #166

Merged
merged 1 commit into from
Dec 3, 2022
Merged

make Discard logger equal to null logger #166

merged 1 commit into from
Dec 3, 2022

Commits on Dec 3, 2022

  1. make Discard logger equal to null logger 

    The original intention was to no treat the discard log sink in a special way.
But it needs special treatment and lacking that in V() led to a bug:
Discard() became different from Discard().V(1).

Adding special detection of the discard logger also helps with the future
logging of context values, because that extra work can be skipped for the
discard logger.

The distinction between null logger (from
go-logr#153) and logr.Discard() was very
minor. Instead of fixing the issue above with checks for both cases, Discard()
now simply returns the null logger.

Performance is a bit better:

name                               old time/op    new time/op    delta
DiscardLogInfoOneArg-36              92.7ns ± 5%    88.2ns ± 3%     ~     (p=0.056 n=5+5)
DiscardLogInfoSeveralArgs-36          239ns ± 0%     231ns ± 1%   -3.40%  (p=0.008 n=5+5)
DiscardLogInfoWithValues-36           240ns ± 1%     236ns ± 3%     ~     (p=0.889 n=5+5)
DiscardLogV0Info-36                   234ns ± 1%     228ns ± 0%   -2.62%  (p=0.008 n=5+5)
DiscardLogV9Info-36                   241ns ± 2%     228ns ± 0%   -5.23%  (p=0.008 n=5+5)
DiscardLogError-36                    264ns ± 1%     230ns ± 0%  -12.78%  (p=0.008 n=5+5)
DiscardWithValues-36                  116ns ± 1%     110ns ± 1%   -5.35%  (p=0.008 n=5+5)
DiscardWithName-36                   2.25ns ± 0%    0.72ns ± 0%  -68.12%  (p=0.008 n=5+5)
FuncrLogInfoOneArg-36                2.13µs ± 2%    2.16µs ± 1%     ~     (p=0.222 n=5+5)
FuncrJSONLogInfoOneArg-36            2.41µs ± 1%    2.42µs ± 1%     ~     (p=0.246 n=5+5)
FuncrLogInfoSeveralArgs-36           4.53µs ± 4%    4.40µs ± 4%     ~     (p=0.151 n=5+5)
FuncrJSONLogInfoSeveralArgs-36       4.71µs ± 8%    4.67µs ± 2%     ~     (p=0.310 n=5+5)
FuncrLogInfoWithValues-36            4.60µs ± 2%    4.61µs ± 4%     ~     (p=0.595 n=5+5)
FuncrJSONLogInfoWithValues-36        4.81µs ± 3%    4.84µs ± 3%     ~     (p=1.000 n=5+5)
FuncrLogV0Info-36                    4.45µs ± 3%    4.55µs ± 3%     ~     (p=0.222 n=5+5)
FuncrJSONLogV0Info-36                4.83µs ± 2%    4.78µs ± 3%     ~     (p=0.548 n=5+5)
FuncrLogV9Info-36                     259ns ± 1%     252ns ± 0%   -2.48%  (p=0.008 n=5+5)
FuncrJSONLogV9Info-36                 258ns ± 1%     252ns ± 1%   -2.52%  (p=0.008 n=5+5)
FuncrLogError-36                     4.97µs ± 1%    4.78µs ± 3%   -3.77%  (p=0.032 n=5+5)
FuncrJSONLogError-36                 5.20µs ± 3%    5.13µs ± 2%     ~     (p=0.548 n=5+5)
FuncrWithValues-36                   1.39µs ± 3%    1.38µs ± 3%     ~     (p=0.690 n=5+5)
FuncrWithName-36                      217ns ± 1%     215ns ± 1%   -0.62%  (p=0.040 n=5+5)
FuncrWithCallDepth-36                 206ns ± 1%     210ns ± 1%   +1.92%  (p=0.008 n=5+5)
FuncrJSONLogInfoStringerValue-36     2.59µs ± 2%    2.59µs ± 2%     ~     (p=1.000 n=5+5)
FuncrJSONLogInfoErrorValue-36        2.61µs ± 2%    2.63µs ± 2%     ~     (p=0.310 n=5+5)
FuncrJSONLogInfoMarshalerValue-36    2.63µs ± 2%    2.63µs ± 1%     ~     (p=0.841 n=5+5)

There's no difference in allocations.

Co-authored-by: Tim Hockin <thockin@google.com>
See go-logr#158 (comment)
    @pohly
    pohly committed Dec 3, 2022
    Configuration menu
    Copy the full SHA
    b90aec2 View commit details
    Browse the repository at this point in the history