Skip to content
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

improvement(query): performance improvement for sorted merge iterator #17733

Closed
wants to merge 1 commit into from

Conversation

foobar
Copy link
Contributor

@foobar foobar commented Apr 14, 2020

Sorted merge iterator has cpu-intensive operations to sort the points
from multiple inputs. Typical queries like SELECT * FROM m GROUP BY *
do not behave well due to the comparison of points though in many cases
it doesn't necessarily have to use the slow path.

This patch adds a shortcut. If each input has a single and unique
series we can just return the points input by input.
The detection of the shortcut introduces slight overhead but the gains
are significant in many slow queries.

Closes #8304

Sorted merge iterator has cpu-intensive operations to sort the points
from multiple inputs. Typical queries like `SELECT * FROM m GROUP BY *`
do not behave well due to the comparison of points though in many cases
it doesn't necessarily have to use the slow path.

This patch adds a shortcut.  If each input has a single and unique
series we can just return the points input by input.
The detection of the shortcut introduces slight overhead but the gains
are significant in many slow queries.
@jacobmarble
Copy link
Member

@dgnorton this looks like a backport of #17596 , which you merged earlier today. Worth adding to 1.8?

@benbjohnson
Copy link
Contributor

@jacobmarble I think it's good to keep versions consistent. We usually merge into master-1.x and backport from there as far back as we need. Anything that goes into 1.7 first should be forward ported to 1.8 and master-1.x so we don't hit performance regressions when users upgrade.

@benbjohnson
Copy link
Contributor

@jacobmarble Sorry, I misread the versions. I thought this was another instance of forward porting.

@jacobmarble
Copy link
Member

This is not a strict backport of #17596. Formatting is off here, and tests are missing. If needed, we can backport the change later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants