Reduced garbage produced on label lookup #445
Closed
Conversation
This file contains hidden or 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
franz1981
force-pushed
the
zero_gc_labels_lookup
branch
from
0f730c1 to
b1717c9
Compare
|
Can you share your benchmark results? I'm particularly interested if the single label case optimistion is worth it. |
|
Yes, sure! |
| @@ -164,9 +228,10 @@ protected SimpleCollector(Builder b) { | |||
| checkMetricName(fullname); | |||
| if (b.help.isEmpty()) throw new IllegalStateException("Help hasn't been set."); | |||
| help = b.help; | |||
| labelNames = Arrays.asList(b.labelNames); | |||
| labelNames = b.labelNames.length == 0 ? Collections.<String>emptyList() : | |||
There was a problem hiding this comment.
This is only called on creation of a metric, it's not on a hot path so I'd not complicate things.
There was a problem hiding this comment.
LabelNames is not used in other contexts? If not, I can remove this one :)
Re the bench I'm running the single threaded to avoid the contention to make less reproducible the results (due to the contention: if I have made it faster, it would hammer more often any contended data structure)
There was a problem hiding this comment.
I need to run the benchmarks in a quiet machine because I'm getting very unstable results!
I will try to run it ASAP and will write them here :)
There was a problem hiding this comment.
The one here should be created and stay alive until the process terminates.
franz1981
force-pushed
the
zero_gc_labels_lookup
branch
from
b1717c9 to
96063c2
Compare
|
MASTER: THIS PR: Looking at the number and thanks to the changes to speed-up |
|
While Re label names lookup perf: With this PR: We just pay more cost CPU-wise on the label names count = 2, but much less GC (0), while with a single label name the perf are improved with no GC at all |
franz1981
force-pushed
the
zero_gc_labels_lookup
branch
from
96063c2 to
e6eb364
Compare
| package io.prometheus.benchmark; | ||
|
|
||
| import io.prometheus.client.Histogram; | ||
| import org.openjdk.jmh.annotations.*; |
There was a problem hiding this comment.
Please be explicit in imports
The ones having garbage are the ones with:
Nope, I have verified several times and are slower for real, but with good reasons... |
| Child c2 = newChild(); | ||
| Child tmp = children.putIfAbsent(labels, c2); | ||
| if (tmp == null) { | ||
| labelNamesPool.set(null); |
There was a problem hiding this comment.
Why do you throwaway the pool here?
There was a problem hiding this comment.
Because if labels is being put into children is better to not sharing it to others that will come later by mean of the pool: the risk is to have the same list in children and pooled.
There was a problem hiding this comment.
Can you add a comment on that?
franz1981
force-pushed
the
zero_gc_labels_lookup
branch
from
e6eb364 to
ac931a4
Compare
Introduced pooling of label Names to reduce garbage in the hot path, updated benchmarks to measure it, improved SimpleCollector creation when are used labels with no label names or with a single element. Introduced a new ArrayList implementation with faster hashCode/equals to allow faster lookups. Signed-off-by: Francesco Nigro <[email protected]>
franz1981
force-pushed
the
zero_gc_labels_lookup
branch
from
ac931a4 to
03576e6
Compare
njhill
added a commit
to njhill/client_java
that referenced
this pull request
Nov 6, 2019
This is an optimization of the SimpleCollector.labels(...) lookups with a similar goal to prometheus#445 and prometheus#459. It has some things in common with those PRs (including overridden fixed-args versions) but aims to provide best of all worlds - zero garbage and higher throughput for all label counts, without any reliance on thread reuse. To achieve this, ConcurrentHashMap is abandoned in favour of a custom copy-on-write linear-probe hashtable. Benchmark results Before: Benchmark Mode Cnt Score Error Units baseline thrpt 20 84731357.558 ± 535745.023 ops/s oneLabel thrpt 20 36415789.294 ± 441116.974 ops/s twoLabels thrpt 20 33301282.259 ± 313669.132 ops/s threeLabels thrpt 20 24560630.904 ± 2247040.286 ops/s fourLabels thrpt 20 24424456.896 ± 288989.596 ops/s fiveLabels thrpt 20 18356036.944 ± 949244.712 ops/s After: Benchmark Mode Cnt Score Error Units baseline thrpt 20 84866162.495 ± 823753.503 ops/s oneLabel thrpt 20 84554174.645 ± 804735.949 ops/s twoLabels thrpt 20 85004332.529 ± 689559.035 ops/s threeLabels thrpt 20 73395533.440 ± 3022384.940 ops/s fourLabels thrpt 20 68736143.734 ± 1872048.923 ops/s fiveLabels thrpt 20 53482207.003 ± 488751.990 ops/s This benchmark, like the prior ones, only tests with a single sequence of labels for each count. It would be good to extend it to cover cases where the map is populated with a larger number of children. Signed-off-by: nickhill <[email protected]>
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.
Introduced pooling of label Names to reduce garbage
in the hot path, updated benchmarks to measure it,
improved SimpleCollector creation when are used
labels with no label names or with a single element.
Introduced a new ArrayList implementation with
faster hashCode/equals to allow faster lookups.