-
Notifications
You must be signed in to change notification settings - Fork 597
Do less Nexting #4753
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
Do less Nexting #4753
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>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Looks good
Definitely interested to see the benchmark results for this approach 👍 |
Signed-off-by: Joe Elliott <number101010@gmail.com>
I spent quite a bit of time trying to find a breakpoint at which to run the unrolled even more exhaustive benches!
|
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_TENANTID
RowNumber.Next()
implementation although I may restore it.BenchmarkIterators
function 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.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]