Skip to content

Split physical_plan_tpch into separate benchmarks #9043

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

simonvandel
Copy link
Contributor

Which issue does this PR close?

Relates to #8638

Rationale for this change

The physical_plan_tpch benchmark was taking a long time to run, without any feedback on the performance of each query.

I think that splitting the benchmark up into a benchmark per query will allow better visibility into how code changes affect individual queries.

What changes are included in this PR?

Move physical_plan_tpch benchmark into multiple physical_plan_tpch_{qn} benchmarks.

Are these changes tested?

Yes, I have verified the benchmarks run.

Are there any user-facing changes?

No

@github-actions github-actions bot added the core Core DataFusion crate label Jan 29, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @simonvandel -- thank you -- this makes sense to me

Could you please also have a single benchmark that runs them all (so that we also have an aggregate number, in addition to the breakdown by individual query)?

@alamb
Copy link
Contributor

alamb commented Jan 29, 2024

The clippy issue was fixed on main, so I think if you merge up from main the CI will pass

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @simonvandel

@alamb alamb merged commit f8495a7 into apache:main Jan 31, 2024
@alamb
Copy link
Contributor

alamb commented Jan 31, 2024

Thanks again @simonvandel

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants