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

logging: do filter last, for improved performance #3875

Merged
merged 2 commits into from
Jan 9, 2023
Merged

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Jan 6, 2023

What this PR does

Previously the code would find out what line it was on and format the time, before deciding that the whole log line should be filtered out. Moving the filter to the end avoids most of this work.

Add a benchmark to demonstrate, and a test to validate that the filtering still works.

Very similar to weaveworks/common#227

name        old time/op    new time/op    delta
DebugLog-4    2.05µs ±10%    0.21µs ±16%  -89.52%  (p=0.008 n=5+5)

name        old alloc/op   new alloc/op   delta
DebugLog-4      520B ± 0%       96B ± 0%  -81.54%  (p=0.008 n=5+5)

name        old allocs/op  new allocs/op  delta
DebugLog-4      7.00 ± 0%      2.00 ± 0%  -71.43%  (p=0.008 n=5+5)

Checklist

  • Tests updated
  • NA Documentation added
  • CHANGELOG.md updated.

Since this is normally filtered out, we want low overhead.
@bboreham bboreham force-pushed the optimise-logging branch 2 times, most recently from 90dfb48 to 6cf0be4 Compare January 7, 2023 12:07
@bboreham bboreham marked this pull request as ready for review January 7, 2023 12:07
@bboreham bboreham requested a review from a team as a code owner January 7, 2023 12:07
Previously the code would find out what line it was on and format the
time, before deciding that the whole log line should be filtered out.
Moving the filter to the end avoids most of this work.

Add a test to validate that the filtering still works.
@pracucci pracucci enabled auto-merge (squash) January 9, 2023 15:51
@pracucci pracucci merged commit 1c3ed0d into main Jan 9, 2023
@pracucci pracucci deleted the optimise-logging branch January 9, 2023 15:51
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.

3 participants