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

Let's talk profiling (again) #296

Open
rubdos opened this issue May 30, 2019 · 4 comments
Open

Let's talk profiling (again) #296

rubdos opened this issue May 30, 2019 · 4 comments

Comments

@rubdos
Copy link

@rubdos rubdos commented May 30, 2019

Hi, and thanks for this great library!

There have been quite a number of suggestions to incorporate profiling in Criterion.rs: e.g. #138, #274. In the former, @bheisler writes:

If you or someone else would like to build and maintain an appropriate Rust profiling engine, I would happily help build an interface to hook it into Criterion.rs to allow users to profile their benchmarks more directly. Until then, however, I'm afraid that using an external profiler will have to be good enough.

To sketch why I would love a more tight integration: I am currently hopping back and forth between commenting out cpuprofiler::PROFILER::... lines and running cargo bench, and uncommenting the line and running cargo bench --profile-time 10. It would thus be nice that the profiler only and automatically gets activated when running criterion in profile mode.


So, I opened this issue to discuss what features such "interface" should contain, and then to see how to design such interface. I'd happily dedicate some of my time to help making this a reality.

Features that I am thinking about:

  • automatic start/stop of the profiling at the beginning of the benchmarking and the end;
  • maybe signalling to the profiler when a single iteration starts and stops. Are there profilers able to use this information yet?
  • signalling the name of the benchmark to the profiler. For example, cpuprofiler requests a filename to dump the gperf data;
  • maybe some kind of integration of the profiling data in the html report? Maybe something for the long-term, or maybe even (far) out of scope. I get that, currently, the html report is currently a write-once system.

In terms of implementation: I think we (or an external crate) could export a trait Profiler that can be implemented by different profiling systems that contains the necessary methods to accomplish the above? That way, no actual profiling code has to be in Criterion.rs.

Thoughts?

@bheisler

This comment has been minimized.

Copy link
Owner

@bheisler bheisler commented Jun 4, 2019

Hey, thanks for the suggestion.

I can see you've put some thought into this, and I think your implementation ideas are reasonable. I was already planning a trait to allow external crates to plug in different measurements and such, so this would fit in well with the designs for 0.3.0 in #261.

Providing hooks to notify the profiler when each sample starts/ends seems to me like the right way to go. That should prevent much of the measurement machinery from showing up in the profiles. Criterion.rs should also provide a path to the benchmark directory so the profiler can store its data there.

As for including the profiler's results in the HTML reports, that would take some careful design. It's tempting to just call a function that returns some HTML text but I suspect that plugins would want a more powerful way to customize the reports so I'd prefer to build something more general. On the other hand, without examples of what they might want it's hard to know what would be appropriate.

@rubdos

This comment has been minimized.

Copy link
Author

@rubdos rubdos commented Jun 5, 2019

I can see you've put some thought into this, and I think your implementation ideas are reasonable. I was already planning a trait to allow external crates to plug in different measurements and such, so this would fit in well with the designs for 0.3.0 in #261.

I'll be reading through that issue. Thanks for the support!

Providing hooks to notify the profiler when each sample starts/ends seems to me like the right way to go. That should prevent much of the measurement machinery from showing up in the profiles. Criterion.rs should also provide a path to the benchmark directory so the profiler can store its data there.

Good thinking on the path. cpuprofiler and the newer gproftools fork both just drop the profile files in the cwd afaict.

As for including the profiler's results in the HTML reports, that would take some careful design. It's tempting to just call a function that returns some HTML text but I suspect that plugins would want a more powerful way to customize the reports so I'd prefer to build something more general. On the other hand, without examples of what they might want it's hard to know what would be appropriate.

Maybe we can postpone this with a future extension of the trait, since this is quite a specialized thing to do. I can see e.g. the profiler hooking into the parameters of a benchmark and highlighting differences somehow in the far future.

@bheisler

This comment has been minimized.

Copy link
Owner

@bheisler bheisler commented Jul 2, 2019

I've added support for profiler hooks to the v0.3.0-dev branch. For now, only a start/stop hook is supported; we can potentially figure out custom reporting in a later version. Specifically, take a look at criterion::profiler::Profiler and the user-guide level documentation in book/src/user_guide/profiling.md.

@rubdos - Could you try implementing a profiler hook to connect to cpuprofiler, to verify that my implementation works for your use case and my documentation make sense? If you want to publish that as a small glue crate after 0.3.0 is released, that'd be great, I'd be happy to link to it.

@rubdos

This comment has been minimized.

Copy link
Author

@rubdos rubdos commented Jul 4, 2019

I currently am not profiling anything, but I hope to have something soon to test this on. Thanks for everything already!

The profiler hook will only take effect when running in --profile-time mode.

Good, I was about to ask that! At first sight, your docs make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.