Skip to content

Enable bench for 9.10 #4512

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

Merged
merged 11 commits into from
Mar 9, 2025
Merged

Enable bench for 9.10 #4512

merged 11 commits into from
Mar 9, 2025

Conversation

soulomoon
Copy link
Collaborator

@soulomoon soulomoon commented Mar 6, 2025

  1. Fix the dependency problem preventing 9.10 from building bench
  2. Enable the bench to build even not for performance label, still need performance label if we need to run it.
  3. bump up to actions/checkout-v4 in bench

@soulomoon soulomoon requested a review from michaelpj as a code owner March 6, 2025 23:12
@soulomoon soulomoon linked an issue Mar 6, 2025 that may be closed by this pull request
@soulomoon soulomoon requested a review from fendor as a code owner March 6, 2025 23:15
@fendor
Copy link
Collaborator

fendor commented Mar 7, 2025

@soulomoon Since hp2pretty seems to be unmaintained, I am wondering whether it would make sense to migrate our benchmarks to eventlog2html. I am unfamiliar with the benchmarks, though, so maybe there is a good reason it is done like this. I suspect that the benchmark suite is rather old, and we could move some of the infrastructure to maintained alternatives.

@soulomoon soulomoon added the performance Issues about memory consumption, responsiveness, etc. label Mar 7, 2025
Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM, this is a great improvement!

@@ -17,7 +17,6 @@ on:
jobs:
pre_job:
runs-on: ubuntu-latest
if: contains(github.event.pull_request.labels.*.name, 'performance')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs to be readded, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I move it down to the run session, so if performance label is provided, we build and run the bench, if not, we only build the bench.

@fendor fendor merged commit 7c92fe2 into master Mar 9, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues about memory consumption, responsiveness, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get benchmarks working on 9.10
2 participants