Closed darrenge closed 1 month ago
My initial thought is "https://garnet.github.io/darrenge/garnet/dev/bench" I think the fork of "darrenge/garnet" is messing things up. The "you" part of this: "https://you.github.io/repo/dev/bench" is confusing me on what should be there.
Hi @darrenge
As far as I can see you have specified auto-push
option to false in your workflow and you are not handling the pushing of the results manually as described at the end of this section
You need to either set auto-push to true or push the changes yourself
Oh yeah ... you are right. Thanks for seeing that and getting back to me. I think the problem was that I was trying to put the Comment Commit feature and the Charting feature into one task. I broke them down into two separate tasks and they are running separately now. I ended up moving to a branch off main (instead of a fork) but I am seeing the same issues there as I do when I work in the fork. I am getting close though. I have 6 different benchmark projects run on linux and windows (so total of 12).
To see my run: https://github.com/microsoft/garnet/actions/runs/11264144295
Issues 1) I only get one of my benchmark projects (Resp.RespTsavoriteStress) to show "Previous" values (for both linux and windows). All the other runs just show the current values. However, these "previous" values for Tsavorite are not really previous as when I do more runs, the "previous" values are the same for all future runs. 2) The charting task fails with a 403 error: remote: Permission to microsoft/garnet.git denied to github-actions[bot]. fatal: unable to access 'https://github.com/microsoft/garnet.git/': The requested URL returned error: 403
Our repo had a gh-pages used by docusarus so i know we can set things up properly. Just need to figure out what I am doing wrong.
Ok ... for some reason, the run on the fork is now passing. https://github.com/darrenge/garnet/actions/runs/11264833381 Not sure why fork works when code is same is in my branch (darrenge/BDNPerf) off main (non fork). 1) On my fork I still don't get true previous 2) On my fork, the Upload to chart passes but I still don't know what my URL would be (https://you.github.io/repo/dev/bench) -- https://darrenge.github.io/repo/dev/bench ???? Doesn't work 3) I look at gh-pages branch and there are merges going in there (on my fork, not on my branch off main) and there is a dev/bench folder.
I suspect that you are missing contents: write
permission on the workflow
Your fork benchmarks are available on https://darrenge.github.io/garnet/dev/bench/
I'll modify the URL pattern to <your_username>.github.io/<repo_name>/dev/bench
which should be more understandable
Thanks again for your responses.
Thanks for showing me that URL. To be honest, I don't understand how it would work that way since my fork is darrenge/garnet. How would my branch off main (not the fork) show it then? There is no place in this URL that shows to be the fork vs a normal branch off main. https://darrenge.github.io/garnet/dev/bench/
As for the permissions, I have those set: https://github.com/darrenge/garnet/blob/main/.github/workflows/ci-bdnbenchmark.yml
There must be some other thing in the YML or in the Github security. We have docusarus for our documentation and it is automatically setting it up using the gh-pages branch as well, so maybe there is something in there that is messing it up.
Hey @darrenge
Your permissions are only set for the changes
job and not the entire workflow. You need to either declare them in the root of the yaml file or declare them for build-run-bdnbenchmark job as well
As per the link, this is not pointing directly to a repository but a deployment to your GitHub Pages. And this is how GH Pages links are structured. Your main URL is https://darrenge.github.io/ and garnet
is pointing to the page deployed from your repo which is named like that (it doesn't matter if it's a fork or not. Only the name of the repo matters).
The reason why it works on your fork and not the main repo is probably due to the default permissions in the repository/org settings. Your fork probably has default read/write permissions for the GITHUB_TOKEN
generated for every CI run. And I assume that on the original the default permissions are read-only
Good call about the default permissions being read only. Thanks again for the help. I will try to do more work on this and let you know how it goes. Thanks again.
I have the charts working on Fork and non Fork. Thanks again.
The last thing I have to ask about (I won't promise but close to promise): For the summary always feature for the git hub action specific run, I am seeing "Current" just fine, but the "previous" isn't being populated. I see the "Download previous benchmark data" step runs and says cache size ~0MB (750 B). Where would that be getting the info for previous?
The previous benchmark data should come from the gh-pages branch /dev/bench/data.js
file
Something is a miss ... here is what I know: 1) This is on my fork but also the my other branch, so it isn't specific to that 2) Charts are working. Proper commits at the bottom of charts etc (https://darrenge.github.io/garnet/dev/bench/) 3) data.js in the gh-pages branch is showing all the up to date commits and data 4) Using the "summary-always" feature - looking at the summaries (https://github.com/darrenge/garnet/actions/runs/11297048682) and the "current" info (commit and data) is correct 4) Looking at that same summary-always, the "Previous" isn't populated and the commit ID isn't even in the data.js file so not sure where getting this previous commit info etc. 5) I compared the output to one of your runs and to mine, and the "Download previous benchmark data" step looks about identical
FYI ... I work on several OSS projects in Microsoft Research. I will be pushing to get people to use this tool in those other projects. Basically, I appreciate what you have done and it will not go to waste on just one project.
Oh... I think I know what the issue is! I've noticed that in your workflow file you haven't set the name param for each of the job in the matrix. The action uses a name to distinguish different benchmarks from each other. And in case of more than one benchmark suite (one matrix combination is essentially one benchmark) it can only find the benchmark results from the last successful run from the previous run.
In order to fix this you need to make sure the name
param is unique for each job run within the matrix so I suggest using a name that looks something like:
name: BDNBenchmark - ${{matrix.test}} ( os: ${{matrix.os}}, framework: ${{matrix.framework}}, configuration: ${{matrix.configuration}} )
And thank you for all those questions! I'll be able to improve the docs after this conversation!
A way to reduce the number of benchmark names would be to run all the test cases in one job, concat the result file, and run the benchmark action on it. This way it would be grouped by one os, framework, and configuration combination but all the test cases would be grouped under one name.
I think this can be considered a bug as if we have the same commit id we should probably merge the results into one entry rather than adding a new one
I think all in one might be too much. Where in the file do you put this line at: name: BDNBenchmark - ${{matrix.test}} ( os: ${{matrix.os}}, framework: ${{matrix.framework}}, configuration: ${{matrix.configuration}} ) It can't be a name of "Store benchmark result for commit comment"
You can put this name under Store benchmark result for charts eg. in line 103
You can take our repo's benchmarkdotnet.yml file as an additional example
FYI ... things are working! :) Thanks again for your help.
One doc thing you should add is regarding this PR: https://github.com/benchmark-action/github-action-benchmark/pull/163
The reason ... we were looking at the scenario (not sure if this is accurate): 1) Joe pushes - BDN perf numbers are inserted (charting and for current vs prev) 2) Sally does a PR (not merged yet) and the BDN Perf Numbers cross threshold and pops perf error 3) Pete does a PR ... would Pete's PR be compared Sally's numbers or Joes's numbers? My thought would be that it would be vs Sally's numbers because the CI ran which would push perf numbers.
I think this would be sufficient: 1) The ability to chart the BDN performance versus time, for every commit to main only, so we can see how main evolved over time, for each benchmark 2) The ability to flag that a given PR, before it is merged, has performance significantly worse than the current latest on main
Closing out this issue as you have answered my questions. If I run into any real issues / bugs, I will file them separately.
Not sure if possible or user error.
I have a fork on the "Garnet" open source project. The fork is "darrenge/garnet".
1) I used example here: https://github.com/benchmark-action/github-action-benchmark?tab=readme-ov-file#charts-on-github-pages-1 to get things set up. 2) I have a branch called gh-pages
The BDNBenchmark ran and the Store Benchmark Result ran and passed. Details here: https://github.com/darrenge/garnet/actions/runs/11187758282/job/31105410031
According to the docs: "After the first workflow run, you will get the first result on https://you.github.io/repo/dev/bench" with the example of https://benchmark-action.github.io/github-action-benchmark/dev/bench/
I can not get this part working where I can see the charts. I hope it is user error and not "can't do on a fork"