-
Notifications
You must be signed in to change notification settings - Fork 545
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
MQE: Add NH support to binary operations #8838
MQE: Add NH support to binary operations #8838
Conversation
For one-to-one mapping.
WIP, no tests yet. Not the most elegant. The merging one side and many-to-one mapping may need rethinking. I'll update the CHANGELOG when it is closer to completed rather than having to resolve merge conflicts constantly. |
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.
Overall looks good to me.
My main concern is performance - have you run the existing benchmarks for binary operations before and after this change?
If the impact is small, then we can continue with this approach. If not, one possibility may be to have fast paths for the common case where each side only has one kind of data (eg. LHS has only floats and RHS has only histograms).
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 somehow managed to add my comments as a review... I'll just publish the review rather than going through github and changing things
Use new InstantVectorSeriesDataIterator And properly assign/re-use slices
This branch vs promql engine:
TL;DR: Some good 30-40% improvements on performance and bytes. |
This branch vs main.
(too long for one comment, posted across multiple) |
|
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.
Very nice stuff 🙂
For one-to-one mapping.
What this PR does
Which issue(s) this PR fixes or relates to
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.