-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore: Add microbenchmarks to lib/datadog/grok #12172
Conversation
✅ Deploy Preview for vector-project canceled.
|
(value, filter) | ||
}, | ||
|(value, filter): (Value, KeyValueFilter)| { | ||
let _ = apply_filter(&value, &filter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use black_box
here or return the value so Criterion handles it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this whole closure might be automatically black boxed? https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#when-should-i-use-criterionblack_box
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, isn't:
If you're writing an un-parameterized benchmark of a function that takes an argument, however, this may be worth considering.
What we are doing here though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. There used to be language about things being auto black-boxed that was more aggressive. I'll add the black box.
Ultimately this one of those pain points with micro-benchmarks I guess. In practice we want the optimizer to remove as much as possible, except not in this case, but that ends up deviating the results more from what we'd see in practice at a macro level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah, that is a tension with benchmarks that I never considered before.
(value, filter) | ||
}, | ||
|(value, filter): (Value, KeyValueFilter)| { | ||
let _ = apply_filter(&value, &filter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
@@ -301,13 +307,30 @@ fn quoted<'a>( | |||
} | |||
} | |||
|
|||
fn re_find<'a, E>(re: &'a Regex) -> impl Fn(&'a str) -> IResult<&'a str, &'a str, E> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, yeah, I do see that nom-regex
takes ownership of the Regex
but seemingly unnecessarily? I'd open an issue but the source doesn't seem to be published somewhere that would be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the source is here? https://github.com/rust-bakery/nom-regex
Considering how little we actually used of nom-regex I'm glad to see a dependency dropped from the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, there it is. And agreed, we were barely using it.
With reference to #10144 and in light of #11849 we now have an understanding that http -> pipelines -> blackhole is significantly bottlenecked in datadog-grok. Unfortunately most of our data indicates we're looking at regex being the prime pain point. This commit does two things: introduces micro-benchmarks for `datadog_grok::filters::keyvalue::apply_filter` -- unfortunately exposing `datadog_grok::filters` from the crate so we can benchmark it -- and improves the performance of said function by +40% in the micro when there is a field delimiter in place. Specifically, we remove the need for nom-regex and avoid cloning a `regex::Regex` instance for each key and each value in a field. Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
Soak Test ResultsBaseline: 7f75d42 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±7.0% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±7.0%: Fine details of change detection per experiment.
Fine details of each soak run.
|
Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
Soak Test ResultsBaseline: 8e10aeb ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±7.0% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±7.0%: Fine details of change detection per experiment.
Fine details of each soak run.
|
Soak Test ResultsBaseline: a7191c8 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±7.0% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. Changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±7.0%:
Fine details of change detection per experiment.
Fine details of each soak run.
|
Soak Test ResultsBaseline: 4ad156f ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±7.0% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. Changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±7.0%:
Fine details of change detection per experiment.
Fine details of each soak run.
|
Soak Test ResultsBaseline: 4ad156f ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±7.0% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±7.0%: Fine details of change detection per experiment.
Fine details of each soak run.
|
With reference to #10144 and in light of #11849 we now have an understanding
that http -> pipelines -> blackhole is significantly bottlenecked in
datadog-grok. Unfortunately most of our data indicates we're looking at regex
being the prime pain point. This commit does two things: introduces
micro-benchmarks for
datadog_grok::filters::keyvalue::apply_filter
--unfortunately exposing
datadog_grok::filters
from the crate so we canbenchmark it -- and improves the performance of said function by +40% in the
micro when there is a field delimiter in place. Specifically, we remove the need
for nom-regex and avoid cloning a
regex::Regex
instance for each key and eachvalue in a field.
Signed-off-by: Brian L. Troutwine brian@troutwine.us