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

Integrate bencher for benchmarks #1300

Closed
tusharmath opened this issue Mar 6, 2024 · 21 comments
Closed

Integrate bencher for benchmarks #1300

tusharmath opened this issue Mar 6, 2024 · 21 comments
Labels
💎 Bounty good first issue Good for newcomers. state: inactive No current action needed/possible; issue fixed, out of scope, or superseded.

Comments

@tusharmath
Copy link
Contributor

The idea here is to integrate Bencher by @epompeii and keep track of performance.

It would be great if we could push data of older commits for the last 2-3 months and visualize it on something like — https://bencher.dev/perf/advent-of-code

@tusharmath tusharmath added the good first issue Good for newcomers. label Mar 6, 2024
@tusharmath
Copy link
Contributor Author

/bounty 50$

Copy link

algora-pbc bot commented Mar 8, 2024

## 💎 $50 bounty • Tailcall Inc.

### Steps to solve:
1. Start working: Comment /attempt #1300 with your implementation plan
2. Submit work: Create a pull request including /claim #1300 in the PR body to claim the bounty
3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

🙏 Thank you for contributing to tailcallhq/tailcall!
🧐 Checkout our guidelines before you get started.
💵 More about our bounty program.

Attempt Started (GMT+0) Solution
🔴 @neo773 Mar 8, 2024, 6:33:24 AM WIP
🟢 @murtaza030 Mar 10, 2024, 6:52:02 AM WIP
🟢 @alankritdabral Mar 10, 2024, 1:19:24 PM WIP

@neo773
Copy link
Contributor

neo773 commented Mar 8, 2024

/attempt #1300

Algora profile Completed bounties Tech Active attempts Options
@neo773    31 tailcallhq bounties
+ 61 bounties from 18 projects
TypeScript, Rust,
JavaScript & more
Cancel attempt

Copy link

algora-pbc bot commented Mar 8, 2024

@neo773: The Tailcall Inc. team prefers to assign a single contributor to the issue rather than let anyone attempt it right away. We recommend waiting for a confirmation from a member before getting started.

@epompeii
Copy link
Contributor

epompeii commented Mar 9, 2024

Thank you for following up @tusharmath!
This is a follow on issue from a discussion on #436

I will create a PR that runs on pushes to the main branch, following the first example shown here: https://bencher.dev/docs/how-to/github-actions/
A working example of this approach is also available here: https://github.com/bencherdev/example/blob/main/.github/workflows/benchmarks.yml
The perf page for that example is here: https://bencher.dev/perf/example

In order for this to work, you all need to have BENCHER_API_TOKEN set as a repository secret.
It will be a bit tedious to add accurate historical data. Though the Bencher API supports backdating reports, getting that historical data would require replaying each past push to main in GitHub Actions in chronological order.
With that said, the PR I'll be posting will be forward looking only.

Currently you all have implemented relative benchmarking, comparing a single run on main to a single run on a PR here: #762
This can also be accomplished with Bencher as well, and this first PR will lay the ground work for you to use more advanced Threshold for detecting performance regressions.

@murtaza030
Copy link

murtaza030 commented Mar 10, 2024

/attempt #1300

Copy link

algora-pbc bot commented Mar 10, 2024

@murtaza030: The Tailcall Inc. team prefers to assign a single contributor to the issue rather than let anyone attempt it right away. We recommend waiting for a confirmation from a member before getting started.

@alankritdabral
Copy link
Contributor

alankritdabral commented Mar 10, 2024

/attempt

Algora profile Completed bounties Tech Active attempts Options
@alankritdabral    1 tailcallhq bounty
+ 1 bounty from 1 project
Cancel attempt

Copy link

algora-pbc bot commented Mar 10, 2024

@alankritdabral: The Tailcall Inc. team prefers to assign a single contributor to the issue rather than let anyone attempt it right away. We recommend waiting for a confirmation from a member before getting started.

@alankritdabral
Copy link
Contributor

alankritdabral commented Mar 10, 2024

Thank you for following up @tusharmath! This is a follow on issue from a discussion on #436

I will create a PR that runs on pushes to the main branch, following the first example shown here: https://bencher.dev/docs/how-to/github-actions/ A working example of this approach is also available here: https://github.com/bencherdev/example/blob/main/.github/workflows/benchmarks.yml The perf page for that example is here: https://bencher.dev/perf/example

In order for this to work, you all need to have BENCHER_API_TOKEN set as a repository secret. It will be a bit tedious to add accurate historical data. Though the Bencher API supports backdating reports, getting that historical data would require replaying each past push to main in GitHub Actions in chronological order. With that said, the PR I'll be posting will be forward looking only.

Currently you all have implemented relative benchmarking, comparing a single run on main to a single run on a PR here: #762 This can also be accomplished with Bencher as well, and this first PR will lay the ground work for you to use more advanced Threshold for detecting performance regressions.

Hey @epompeii i saw your comment and created a 1367, i took a little bit inspiration from 3849. I would be grateful if you could have a look to the pr.

@epompeii
Copy link
Contributor

@tusharmath I have create a pull request to add Bencher to track the performance of the main branch in the tailcall CI: #1441

I very much appreciate the generous offer of a bounty. My goal in doing these sorts of integrations is to spread the word about Bencher. All that I ask in return is that you help spread the word in your own way, however large or small 😃

@epompeii
Copy link
Contributor

epompeii commented Mar 19, 2024

I'm aware of the failure occurring here: https://github.com/tailcallhq/tailcall/actions/runs/8332008049/job/22800159465#step:5:1
And I'm investigating.

Reverted in the mean time by: #1505

@epompeii
Copy link
Contributor

epompeii commented Apr 12, 2024

@tusharmath I'm sorry for the delay getting back to this and my failure to fully test #1441, that is totally on me.

The reason for the delay is that I was heads down working on Bencher v0.4.5. This new version completely refactors the way that branch selection is handled and it deprecates some of the options that I would have used here. I wanted to finish that work before proceeding so I didn't add options that were going to be deprecated a few days later.
Plus, the new way of doing things is much more elegant!

With that said, it looks like @alankritdabral has done a pretty nice job adding Bencher in #1679. Though he is using the now deprecated options/way of doing things.
I have left a review on the PR for the things that need to be updated: #1679 (comment)
If he is willing to update things, then using his PR is alright with me. Otherwise, I will do my best to get this knocked out 😃

@tusharmath
Copy link
Contributor Author

Thanks! Would love to see this integrated into Tailcall soon.

@alankritdabral
Copy link
Contributor

@tusharmath I'm sorry for the delay getting back to this and my failure to fully test #1441, that is totally on me.

The reason for the delay is that I was heads down working on Bencher v0.4.5. This new version completely refactors the way that branch selection is handled and it deprecates some of the options that I would have used here. I wanted to finish that work before proceeding so I didn't add options that were going to be deprecated a few days later. Plus, the new way of doing things is much more elegant!

With that said, it looks like @alankritdabral has done a pretty nice job adding Bencher in #1679. Though he is using the now deprecated options/way of doing things. I have left a review on the PR for the things that need to be updated: #1679 (comment) If he is willing to update things, then using his PR is alright with me. Otherwise, I will do my best to get this knocked out 😃

Thankyou for your review means a lot ❤️ . Almost done with the changes!

@alankritdabral
Copy link
Contributor

@tusharmath @epompeii done with the changes. #1679 is good to go.

@epompeii
Copy link
Contributor

Okay, lets finally get this knocked out!
@tusharmath I have created a new pull request: #1725
Again, my apologies for the hubris to not fully test things out the first time around. 🤦🏽‍♂️

Unfortunately, after taking a closer look at @alankritdabral PR #1679, I noticed some fundamental security vulnerabilities in his approach for handling PRs from forks. Bencher has built in defenses that probably would have prevented you all from getting pwn requested, but it would have broken your build (again). So I just went ahead and got things implemented as recommended in the docs: https://bencher.dev/docs/how-to/github-actions/#pull-requests-from-forks

Copy link

github-actions bot commented Jun 5, 2024

Action required: Issue inactive for 30 days.
Status update or closure in 7 days.

@github-actions github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jun 5, 2024
@epompeii
Copy link
Contributor

@tusharmath is there anything else needed to close this out, now that #1725 has merged?

@github-actions github-actions bot removed the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jun 10, 2024
Copy link

Action required: Issue inactive for 30 days.
Status update or closure in 7 days.

@github-actions github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jul 28, 2024
Copy link

github-actions bot commented Aug 4, 2024

Issue closed after 7 days of inactivity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty good first issue Good for newcomers. state: inactive No current action needed/possible; issue fixed, out of scope, or superseded.
Projects
None yet
5 participants