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

Add experimental support for perf (via LinuxPerf.jl) #347

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Zentrik
Copy link
Contributor

@Zentrik Zentrik commented Dec 28, 2023

Replaces #325 (closes #325)

Depends on JuliaPerf/LinuxPerf.jl#38.

This pr adds the ability to run perf at the end of a benchmark run for at least a microsecond. It's disabled by default. If enabled it stores the results (adds them to TrialEstimate) with the rest of the benchmark data.

The default is to only measure instructions and branch-instructions as these seem to be the only things non noisy enough to give stable results for the short runs we do. Nor do we need to measure cycles or task-clock given we are running a benchmark beforehand anyways.

For comparison benchmarking BaseBenchmarks.SUITE[["inference", "allinference", "many_local_vars"]] the percentage difference between the max and min minimum time over 5 runs is 1.25% whilst it's 0.002% for instruction count. Given on Nanosoldier the same benchmark is changing about 5-10% every few runs (see here), this seems like it's going to work very well

@Zentrik

This comment was marked as outdated.

@Zentrik

This comment was marked as resolved.

@Zentrik

This comment was marked as resolved.

@Zentrik Zentrik changed the title [WIP] Add experimental support for perf (via LinuxPerf.jl) Add support for perf (via LinuxPerf.jl) Dec 30, 2023
@Zentrik Zentrik changed the title Add support for perf (via LinuxPerf.jl) Add experimental support for perf (via LinuxPerf.jl) Dec 30, 2023
Zentrik added a commit to Zentrik/LinuxPerf.jl that referenced this pull request Dec 30, 2023
@Zentrik

This comment was marked as outdated.

src/trials.jl Show resolved Hide resolved
@Zentrik Zentrik marked this pull request as ready for review December 30, 2023 14:44
@DilumAluthge
Copy link
Member

I haven't added tests, but given the CI doesn't have perf available, not sure how useful it would be.

We might need to set up Buildkite CI on this repo. @vchuravy @staticfloat

src/trials.jl Show resolved Hide resolved
@Zentrik Zentrik force-pushed the linux-perf branch 2 times, most recently from f27a5de to bbfb733 Compare January 7, 2024 23:31
@Zentrik

This comment was marked as outdated.

src/execution.jl Outdated Show resolved Hide resolved
@Zentrik Zentrik force-pushed the linux-perf branch 3 times, most recently from 6b2884f to b1bc53c Compare May 2, 2024 21:29
@willow-ahrens
Copy link
Collaborator

I'm not sure I have the bandwidth as a maintainer of this package to support this kind of infrastructure directly in benchmarktools.jl. Is there a way that we can support this inside LinuxPerf.jl rather than in BenchmarkTools.jl? Maybe a separate run_perf function that runs the benchmarks in perf?

@willow-ahrens
Copy link
Collaborator

willow-ahrens commented May 9, 2024

it seems like too much overhead to me to support separate CI for this just to support this feature here, especially if it's possible to support this in linuxperf.jl. Could linuxperf support a separate run_perf function that runs a BenchmarkTools Benchmark through perf?

Especially if we need special CI to run some software, it seems much easier to support this kind of feature in that repo, rather than this one.

@Zentrik
Copy link
Contributor Author

Zentrik commented May 18, 2024

I think if I can get this to a low enough maintenance overhead for you it would be best to add perf profiling in BenchmarkTools. I think we can just remove the extra CI and make it clear that this is experimental. I don't think there's much need to make sure the profiling works on CI and users can easily run their own tests. Let me know if there's anything else I can do to lower the overhead or if it's not going to be possible for the overhead to be low enough.

If you care why I don't think it makes sense to put this functionality in LinuxPerf or BaseBenchmarks, I've explained below.
So running perf on samplefunc seems to work ok, it has fairly high overhead but it seems to be not much noisier than this pr which is what really matters. However, actually integrating this in LinuxPerf or BaseBenchmarks seems difficult as presumably they would have to overload BenchmarkTools._run so that they can run perf whilst the function is still hot when benchmarking a BenchmarkGroup. I don't think we would want to do this by default for people using LinuxPerf so if I was to do this I would probably put it in BaseBenchmarks which doesn't have CI anyways. Also, they would have to setup a way to save and load the perf profiling results.

@willow-ahrens
Copy link
Collaborator

Could we add something like a prehook and a posthook argument to the run function, so that other packages can export their own versions of run with special behavior, such as bprofilerun, bprofileviewrun, bperfrun? Is there a way to make them nest?

@willow-ahrens
Copy link
Collaborator

willow-ahrens commented May 24, 2024

I'm imagining something like:

run_perf(args...; kwargs..., posthook=f) = run(args...; kwargs..., posthook=(b)->(run_perf_stuff(b); f(b)))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants