-
Notifications
You must be signed in to change notification settings - Fork 259
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
Comments
/bounty 50$ |
|
/attempt #1300
|
@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. |
Thank you for following up @tusharmath! I will create a PR that runs on pushes to the In order for this to work, you all need to have Currently you all have implemented relative benchmarking, comparing a single run on main to a single run on a PR here: #762 |
/attempt #1300 Options |
@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. |
/attempt
|
@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. |
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. |
@tusharmath I have create a pull request to add Bencher to track the performance of the 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 😃 |
I'm aware of the failure occurring here: https://github.com/tailcallhq/tailcall/actions/runs/8332008049/job/22800159465#step:5:1 Reverted in the mean time by: #1505 |
@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.
|
Thanks! Would love to see this integrated into Tailcall soon. |
Thankyou for your review means a lot ❤️ . Almost done with the changes! |
@tusharmath @epompeii done with the changes. #1679 is good to go. |
Okay, lets finally get this knocked out! 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 |
Action required: Issue inactive for 30 days. |
@tusharmath is there anything else needed to close this out, now that #1725 has merged? |
Action required: Issue inactive for 30 days. |
Issue closed after 7 days of inactivity. |
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
The text was updated successfully, but these errors were encountered: