Do less Nexting - #4753
Merged
Merged
Conversation
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
joe-elliott
requested review from
electron0zero,
ie-pham,
javiermolinar,
mapno,
mdisibio,
stoewer,
yvrhdn and
zalegrala
as code owners
February 25, 2025 20:30
Signed-off-by: Joe Elliott <number101010@gmail.com>
3 tasks
stoewer
reviewed
Feb 26, 2025
Contributor
|
Looks good
Definitely interested to see the benchmark results for this approach 👍 |
Signed-off-by: Joe Elliott <number101010@gmail.com>
Collaborator
Author
|
I spent quite a bit of time trying to find a breakpoint at which to run the unrolled even more exhaustive benches! |
stoewer
approved these changes
Feb 28, 2025
mattdurham
pushed a commit
to mattdurham/tempo
that referenced
this pull request
Jun 18, 2026
* this is garbage Signed-off-by: Joe Elliott <number101010@gmail.com> * filtery stuff Signed-off-by: Joe Elliott <number101010@gmail.com> * fix Signed-off-by: Joe Elliott <number101010@gmail.com> * max def everywhere Signed-off-by: Joe Elliott <number101010@gmail.com> * clean up benches Signed-off-by: Joe Elliott <number101010@gmail.com> * clean up Signed-off-by: Joe Elliott <number101010@gmail.com> * remove vendor chagnes Signed-off-by: Joe Elliott <number101010@gmail.com> * changelog Signed-off-by: Joe Elliott <number101010@gmail.com> * add details about bench env vars Signed-off-by: Joe Elliott <number101010@gmail.com> --------- Signed-off-by: Joe Elliott <number101010@gmail.com>
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
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.
What this PR does:
Passes the max definition level into the
RowNumber.Next()function to reduce the amount of required work. This allows for inlining and less work done when iterating at lower definition levels. This does mean that heavy iteration at higher definition levels is a bit slower. Putting up this PR b/c it shows a lot of promise, but I still want to test a version that reverts to the unrolled version for higher max definition levels.This PR is a bit messy 😬. Look at
RowNumber.Next(I can't link to it b/c it's in the collapsed iter file). All substantive changes flow from this.Other Changes:
BENCH_BLOCKID,BENCH_PATH,BENCH_TENANTIDRowNumber.Next()implementation although I may restore it.BenchmarkIteratorsfunction to better test raw iterator combinations. I find it useful for testing, but am having issues translating perf improvements found here to TraceQL queries.Generally I feel like more work could be done on our benchmarks to better guide us when making performance improvements. This is a first step.
really long exhaustive benchmarks
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]