chore: Introduce a new route transform benchmark #11697
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
|
✔️ Deploy Preview for vector-project canceled. 🔨 Explore the source changes: 29fb1ae 🔍 Inspect the deploy log: https://app.netlify.com/sites/vector-project/deploys/6223e2fc1a436f000715a7b8 |
github-actions
bot
added
the
domain: core
blt
force-pushed
the
transform_route_benchmarks
branch
from
c7ad21c
to
49e80a2
Compare
jszwedko
reviewed
Mar 5, 2022
jszwedko
reviewed
Mar 5, 2022
| log_schema_definition: None, | ||
| }); | ||
| } | ||
| let output_buffer: TransformOutputsBuf = TransformOutputsBuf::new_with_capacity(outputs, 10); |
There was a problem hiding this comment.
Looking at the route transform it looks like it uses a capacity of whatever number of routes there are. Should we do the same here by moving this into the setup?
There was a problem hiding this comment.
Actually, I was looking at the tests 🤦
There was a problem hiding this comment.
Still not sure where this 10 comes from though, would it make sense to match whatever the topology does for this?
There was a problem hiding this comment.
Ah, it turns out this doesn't actually propagate to the OutputBuffer at all, the passed capacity. In the event we have a named output it is created like OutputBuffer::default(). I'll experiment with actually passing the capacity in a follow-up PR.
There was a problem hiding this comment.
10 is just an arbitrary number I typed out. I'm not sure what the topology actually does here, off the top of my head. I think, considering the nature of the transform's apparent bottlenecks, this value doesn't matter all that much.
|
jszwedko
approved these changes
Mar 5, 2022
blt
added a commit
that referenced
this pull request
Mar 5, 2022
This commit is a follow-up to #11697. We don't actually need to iterate an `IndexMap` repeatedly here as we never actually use the map capabilities of the conditions field: a vec is enough. This is worth a small boost in the micro but I suspect nothing in the macro. If we are to actually optimize route we'll probably need to run multiple copies of them, run an `EventArray` through it and/or dramatically improve the behavior of `condition.check`. Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
blt
force-pushed
the
transform_route_benchmarks
branch
from
89d4ecc
to
cbe3f1d
Compare
blt
added a commit
that referenced
this pull request
Mar 5, 2022
This commit is a follow-up to #11697. We don't actually need to iterate an `IndexMap` repeatedly here as we never actually use the map capabilities of the conditions field: a vec is enough. This is worth a small boost in the micro but I suspect nothing in the macro. If we are to actually optimize route we'll probably need to run multiple copies of them, run an `EventArray` through it and/or dramatically improve the behavior of `condition.check`. Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
blt
force-pushed
the
transform_route_benchmarks
branch
from
cbe3f1d
to
65941f5
Compare
Soak Test ResultsBaseline: 9569a10 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 ±5% 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 >= ±5%: 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: 9569a10 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 ±5% 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 >= ±5%: Fine details of change detection per experiment.
Fine details of each soak run.
|
blt
added a commit
that referenced
this pull request
Mar 5, 2022
This commit is a follow-up to #11697. We don't actually need to iterate an `IndexMap` repeatedly here as we never actually use the map capabilities of the conditions field: a vec is enough. This is worth a small boost in the micro but I suspect nothing in the macro. If we are to actually optimize route we'll probably need to run multiple copies of them, run an `EventArray` through it and/or dramatically improve the behavior of `condition.check`. Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
blt
added a commit
that referenced
this pull request
Mar 6, 2022
This commit is a follow-up to #11697. We don't actually need to iterate an `IndexMap` repeatedly here as we never actually use the map capabilities of the conditions field: a vec is enough. This is worth a small boost in the micro but I suspect nothing in the macro. If we are to actually optimize route we'll probably need to run multiple copies of them, run an `EventArray` through it and/or dramatically improve the behavior of `condition.check`. Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
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.
This commit introduces a new benchmark for the route transform with the
understanding that we are, in some cases, bottlenecked through this
transform. See #11688 as an
example. Possible contributor to #10144.
Signed-off-by: Brian L. Troutwine brian@troutwine.us