optimize Collector.sanitizeMetricName #777
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Flavio Brasil <fwbrasil@gmail.com>
984d75a to
7f0f7a3
Compare
|
Hi @fstab, I'm sorry to ping you directly. If you have an opportunity to take a look at this PR, we wanted to know if it's something that seems to make sense for you (it's ok if you can't review/merge atm). If not, we're going to try to optimize it in |
|
Hi, thanks for the PR. I am planning to have a look this weekend, didn't find the time during the week. |
fstab
approved these changes
May 2, 2022
|
Thanks a lot @fwbrasil. I merged it. |
1 task
1 task
hezhangjian
pushed a commit
to apache/pulsar
that referenced
this pull request
Jul 14, 2022
### Motivation prometheus client 0.16.0 contains some approvements that we can benefit from. Thanks for @dave2wave @michaeljmarshall the reminder and pointing out. > [ENHANCEMENT] Reduce the number of core threads in HTTPServer from 5 to 1. The HTTPServer will still start up to 5 threads on demand if there are parallel requests, but it will use only 1 thread as long as requests are sequential (prometheus/client_java#786). [ENHANCEMENT] Optimize metric name sanitization: Replace the regular expression with a hard-coded optimized algorithm to improve performance (prometheus/client_java#777). Thanks @fwbrasil See https://github.com/prometheus/client_java/releases ### Modifications Bump prometheus client version from 0.15.0 to 0.16.0 ### Documentation Check the box below or label this PR directly. Need to update docs? - [x] `doc-not-needed` dependency updates, no need doc
Jason918
pushed a commit
to apache/pulsar
that referenced
this pull request
Sep 2, 2022
### Motivation prometheus client 0.16.0 contains some approvements that we can benefit from. Thanks for @dave2wave @michaeljmarshall the reminder and pointing out. > [ENHANCEMENT] Reduce the number of core threads in HTTPServer from 5 to 1. The HTTPServer will still start up to 5 threads on demand if there are parallel requests, but it will use only 1 thread as long as requests are sequential (prometheus/client_java#786). [ENHANCEMENT] Optimize metric name sanitization: Replace the regular expression with a hard-coded optimized algorithm to improve performance (prometheus/client_java#777). Thanks @fwbrasil See https://github.com/prometheus/client_java/releases ### Modifications Bump prometheus client version from 0.15.0 to 0.16.0 ### Documentation Check the box below or label this PR directly. Need to update docs? - [x] `doc-not-needed` dependency updates, no need doc (cherry picked from commit 948000b)
lhotari
pushed a commit
to apache/pulsar-sql
that referenced
this pull request
Oct 18, 2024
### Motivation prometheus client 0.16.0 contains some approvements that we can benefit from. Thanks for @dave2wave @michaeljmarshall the reminder and pointing out. > [ENHANCEMENT] Reduce the number of core threads in HTTPServer from 5 to 1. The HTTPServer will still start up to 5 threads on demand if there are parallel requests, but it will use only 1 thread as long as requests are sequential (prometheus/client_java#786). [ENHANCEMENT] Optimize metric name sanitization: Replace the regular expression with a hard-coded optimized algorithm to improve performance (prometheus/client_java#777). Thanks @fwbrasil See https://github.com/prometheus/client_java/releases ### Modifications Bump prometheus client version from 0.15.0 to 0.16.0 ### Documentation Check the box below or label this PR directly. Need to update docs? - [x] `doc-not-needed` dependency updates, no need doc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Some libraries resolve Prometheus metrics dynamically and use
Collector.sanitizeMetricNamefor that. Ideally, this method shouldn't be in the hot path but it seems worth optimizing it for libraries that provide dynamic metric resolution like https://github.com/clj-commons/iapetos so the performance penalty isn't a concern for them.I noticed this issue while profiling an application that uses
iapetos. The regex execution is currently the main cost of the application:Including being the source of a large portion of the invoke interface dispatches:
Solution
Optimize the
Collector.sanitizeMetricNamemethod to avoid the regex engine and use a simple for-loop.Results
I've created a new benchmark for this method. Results:
Before
After