waycrate / wayshot

Mirrored at https://git.sr.ht/~shinyzenith/wayshot | screenshot tool for wlroots based compositors implementing zwlr_screencopy_v1
https://crates.io/crates/wayshot
BSD 2-Clause "Simplified" License
108 stars 18 forks source link

Add automatic benchmarks to Github Actions #103

Open nukelet opened 4 months ago

nukelet commented 4 months ago

This is a WIP attempt at implementing automatic benchmarks in the project's CI/CD pipeline (#45). This should prove useful for catching potential performance regressions in the future.

I tried to achieve this by launching a headless sway session (using WLR_BACKENDS=headless) with a config that creates a virtual, headless output that wayshot can capture. We then run wayshot to take a screenshot and use flamegraph to plot a heatmap of the functions in wayshot that used the most CPU time, and also run a few benchmarks with hyperfine. The results of both are then saved as artifacts for later inspection.

Currently there are three main issues:

Here is an example of a successful CI/CD run with these changes, as well as the flamegraph/hyperfine artifacts produced: https://github.com/nukelet/wayshot/actions/runs/8261924814

I'd like to get some feedback on whether this looks like a step in the right direction or if another approach would be better.

Shinyzenith commented 4 months ago

Hi can you point your pr to freeze feat Andreas It currently points to main which is not our current dev branch.

Shinyzenith commented 4 months ago

Since it was Andreas who introduced me to flamegraph and suggested to have a testing suite, let me just ping them to get their opinion on it too.

CC: @AndreasBackx

AndreasBackx commented 4 months ago

This would be a great direction imo. I agree that running this on CI/CD might not be super accurate, though I would say let's give it a shot and see how stable those measurements are for now. This would definitely be better than nothing.

I haven't got much experience with benchmarking tools out there, though I would again focus on what's free and simply experiment. We can make this a non-blocking signal in CI/CD or elsewhere. Maybe make a bot automatically post the information on a PR. Though even simply making this easy to run already is hugely impactful.

Would be nice to see benchmarks for a combination of screen resolutions, scalings, and amount of them. I'd say a permutation of the following would already be plenty:

This might be too many permutations though we can look at the most important ones. Also would be interesting to see this for maybe 8K (7680x4320) or 4K super ultrawides (7680x2160).

Finally, would be nice to have this easily visible when making PRs. I don't know if GitHub actions makes it easy to automatically give some feedback or not.

nukelet commented 4 months ago

Hi again!

Finally, would be nice to have this easily visible when making PRs. I don't know if GitHub actions makes it easy to automatically give some feedback or not.

Just pushed a workflow to automatically comment the benchmark results on PRs. Currently it will comment on every PR sync (new commit, force push, etc), so I'm not sure if it would be too spammy, but we can adjust that later if needed. I'm also working on the other test cases you suggested :-)

nukelet commented 4 months ago

Also, I spent way longer than I'd like to admit trying to figure out a way to get the bot to post the flamegraph heatmap SVG as an image in the comment, but that proved quite tricky without hosting the image outside of Github. Maybe we could convert it to PNG and upload to Imgur, or just upload the raw SVG somewhere (since Github apparently has support for displaying them)?

Shinyzenith commented 4 months ago

GitHub has a native CDN right? I believe it's under the usercontent subdomain. Can we just keep the svg data in memory and pass it to the cdn?

nukelet commented 4 months ago

It does, but AFAIK it only works when you manually add a file through the browser, similarly to this (I don't think we can reproduce this in CI/CD since it would at the very least require user authentication).

I just had the idea of uploading the SVG to a gist (since it's just code anyway) and then pointing to it in the PR comment. Let's see if this works. :D

Also, had to fix a silly permission error that caused the last actions run to fail.

Shinyzenith commented 4 months ago

Feel free to contact me on discord. I'm willing to buy some hosting to make this easier for you. Don't want to spam GitHub and potentially get blocked.