Skip to content

Comments

Use strings.Replacer instead of hand-rolled escapeString#48

Merged
fabxc merged 1 commit intoprometheus:masterfrom
lmb:fix-escapestring
Jul 26, 2016
Merged

Use strings.Replacer instead of hand-rolled escapeString#48
fabxc merged 1 commit intoprometheus:masterfrom
lmb:fix-escapestring

Conversation

@lmb
Copy link
Contributor

@lmb lmb commented Jul 25, 2016

This reduces run-time (since replacement is done via table lookup) and memory consumption (since strings that do not need replacing are returned as they are).

name      old time/op    new time/op    delta
Create-4    66.9µs ± 3%    55.0µs ±12%  -17.82%  (p=0.008 n=5+5)

name      old alloc/op   new alloc/op   delta
Create-4    19.2kB ± 0%    13.7kB ± 0%  -28.44%  (p=0.008 n=5+5)

name      old allocs/op  new allocs/op  delta
Create-4       617 ± 0%       498 ± 0%  -19.29%  (p=0.008 n=5+5)

default:
result.WriteRune(c)
}
r := escapeReplacer
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can just directly call escapeReplacer.Replace(v) etc.

@fabxc
Copy link
Contributor

fabxc commented Jul 25, 2016

Thanks, should be good after comment.

This reduces run-time (since replacement is done via table lookup) and memory consumption (since strings that do not need replacing are returned as they are).

    name      old time/op    new time/op    delta
    Create-4    66.9µs ± 3%    55.0µs ±12%  -17.82%  (p=0.008 n=5+5)

    name      old alloc/op   new alloc/op   delta
    Create-4    19.2kB ± 0%    13.7kB ± 0%  -28.44%  (p=0.008 n=5+5)

    name      old allocs/op  new allocs/op  delta
    Create-4       617 ± 0%       498 ± 0%  -19.29%  (p=0.008 n=5+5)
@lmb
Copy link
Contributor Author

lmb commented Jul 25, 2016

Yup, done.

@lmb lmb mentioned this pull request Jul 26, 2016
@fabxc
Copy link
Contributor

fabxc commented Jul 26, 2016

Thank you 👍

@fabxc fabxc merged commit bc0a446 into prometheus:master Jul 26, 2016
gouthamve pushed a commit to gouthamve/common that referenced this pull request Jul 22, 2018
Motivating example is to not excessively log the same http 4xx code over and over from the same ip,
as this doesn't add info and bloats our log volume.

This requires some abuse of logrus to work, but we do this via a formatter which can either emit the original message,
or the empty string to log no message (this is harmless). Later, a seperate goroutine will emit a new log
that logs the original log and how many times it occurred.

We always log the first time we see a given message, then start a timer. If we see the message again at least once in that
time frame, we log at the end of the time frame how many times.
Note this means we can say "repeated once", which is a little confusing but preferable to displaying the original message
unchanged but time delayed, which can be very misleading as there's no indication of the time delay.
alanprot pushed a commit to alanprot/common that referenced this pull request Mar 15, 2023
…)"

This reverts commit a23f449.

It has never de-duplicated a single line for us in production.
alanprot pushed a commit to alanprot/common that referenced this pull request Mar 15, 2023
Revert "logging: Add ability to deduplicate log messages (prometheus#48)"
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