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

tprof :) #439

Open
ruslandoga opened this issue Nov 27, 2024 · 5 comments · May be fixed by #441
Open

tprof :) #439

ruslandoga opened this issue Nov 27, 2024 · 5 comments · May be fixed by #441

Comments

@ruslandoga
Copy link

ruslandoga commented Nov 27, 2024

👋

Thoughts on adding tprof profiler? It can also potentially allow for more accurate memory measurements (just from reading the docs, I haven't actually used it yet).

Right now using profile_after: :tprof fails with

** (Benchee.Profile.Benchee.UnknownProfilerError) Got an unknown ':tprof' built-in profiler.
    (benchee 1.3.1) lib/benchee/profile.ex:124: Benchee.Profile.profiler_to_module/1
    (benchee 1.3.1) lib/benchee/profile.ex:91: Benchee.Profile.do_profile/4
    (benchee 1.3.1) lib/benchee/profile.ex:85: Benchee.Profile.profile/2
    bench/logger_json_overhead.exs:83: (file)

I'd be happy to contribute! (But again, I haven't used it yet so I am not sure how hard it would be to support)

@PragTob
Copy link
Member

PragTob commented Nov 27, 2024

👋 Thanks for coming here! I was gonna say it may be hard, but it may actually be trivial. The reason being, profilers are built on the back of the accompanying mix tasks. But such a mix tasks exists for tprof: https://hexdocs.pm/mix/Mix.Tasks.Profile.Tprof.html

So this might be so trivial, I might just try it straight up :)

That said, I've also been thinking about more general profiler support i.e. eflambe as I saw someone use it in a talk recently :)

Right let me make an issue about that!

@ruslandoga
Copy link
Author

ruslandoga commented Nov 27, 2024

Re eflambe / FlameOn / etc., I actually think they might benefit from the new tracer too:

Right now they use a slightly hack-y approach of mocking the profiled functions which I think can and should be avoided.

So if my suggestions there are accepted, it would be equivalent to using tprof but with some extra wrappers on top. Also neither the currently supported Benchee profilers (fprof is kind of there, but it's too verbose) nor eflambe propagate trace context to the gen:called processes, which results in confusing reports. Like, when setting profile_after: true in Benchee, and profiling a function that calls a GenServer, we wouldn't see what was happening in the GenServer process.

@PragTob
Copy link
Member

PragTob commented Nov 27, 2024

@ruslandoga sorry eflambe/more generic plugin support for tracers was just a side idea/you reminded me I wanted to open up #440 :)

Thanks for your input/thoughts about them - I'm not very familiar with them!

PragTob added a commit that referenced this issue Nov 27, 2024
* Fixes #439
* also fixes one of the samples not being an .exs 😁
  * also updates it and includes tprof
* might consider only adding it on elixir 1.17+ but may be too much
* still needs to be added to docs
@PragTob PragTob linked a pull request Nov 27, 2024 that will close this issue
@PragTob
Copy link
Member

PragTob commented Nov 27, 2024

@ruslandoga yeah was easy, it's over at #441 I'll need to add some docs and maybe a fail-safe or 2. Are you fine running off that branch or later main or would you need a release? I'd normally wait with a release :)

@ruslandoga
Copy link
Author

ruslandoga commented Nov 27, 2024

I'm totally fine running off of that branch. Thank you!

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 a pull request may close this issue.

2 participants