Convert escapeMetricName to use strings.Builder #217
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This converts
escapeMetricName
to usestrings.Builder
instead of allocating a byte array, filling it and then converting it to a string. This also optimizes for the case where the metricName is already valid, and in that case, it just returns the original string.This reduces memory allocations from 2 per call to 1 per call in the case when the string does need to be escaped, and reduces it to zero memory allocations when the string is already valid.
As discussed in #197,
escapeMetricName
is called a lot, and even after the improvements there, we still see this pretty far up the list in terms of cpu usage in profiling.Benchmark before:
Benchmark after:
As you can see from the benchmarks, the common case where no escaping needs to be done is about 3-4x faster.