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

implement heap profiler #45

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

Conversation

YangKeao
Copy link
Member

Fix #44. It seems that the protobuf generation still has a lot of bugs :(.

The flamegraph output works as expected. However, the svg generated by go tool pprof through a protobuf is too simple. The recursive part in the example fails to display in the svg.

The example code snippet is in /examples/heap_profiler.rs. @mineralres Are you willing to take a look and find the problem of protobuf? Feel free to ask any questions about this project. I'm not confident about the code quality (as I have found that it's hard to extend this feature without breaking the compatibility), so if you are anxious about the codes, I'm on your side 😭 .

pprof.zip is the generate flamegraph and protobuf.

Signed-off-by: Yang Keao keao.yang@yahoo.com

Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@YangKeao YangKeao force-pushed the heap-profile branch 3 times, most recently from 9dd46b3 to 72ad5c5 Compare December 15, 2020 11:09
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@mineralres
Copy link

@YangKeao I can have a try. Before that , i had to spend some time to be familiar with the whole pprof-rs project.

@YangKeao
Copy link
Member Author

I'm busy with other projects (actually, two projects 😭 ), so this PR may take several weeks to land. There are a lot of things left:

  • set title and units for heap profiler output
  • reorganize the code structure

@mkmik
Copy link

mkmik commented Aug 16, 2021

Just heads-up: I was playing with a similar approach in https://github.com/mkmik/heappy

I'd like to contribute it upstream here once I figure out whether it works well in practice.

@YangKeao
Copy link
Member Author

YangKeao commented Aug 17, 2021

Just heads-up: I was playing with a similar approach in https://github.com/mkmik/heappy

I'd like to contribute it upstream here once I figure out whether it works well in practice.

Thanks 👍 . I could provide some thoughts about this PR and I hope it could help you 🍻 .

  1. One of the most annoying problem is to avoid recursion while writing the collector. The recursion could be caused by the allocation of the same allocator: malloc -> collector::record -> malloc -> collector::record .... Using a thread local flag to store whether this process is inside the malloc could be a solution (here is the pseudo-code):

    fn malloc() {
      in_malloc = true;
      if !in_malloc {
        collector::record
      }
      in_malloc = false;
    }

    Another possible solution is to use collections with the inner allocator, which depends on the language feature Allocators, take III rust-lang/rfcs#1398. The disadvantages of these solutions are that they fail to record the collector memory footprint.

  2. I have to say the methods and structs to generate report (like flamegraph and protobuf) in pprof-rs are not well designed 🤦‍♂️ . If you need any help to make it work, feel free to contact me and I would try my best to help.

@kwiesmueller
Copy link

What is the status of this? Getting heap profiling to work would be great.

@Rperry2174
Copy link

We also have gotten a lot of requests for this at Pyroscope :) -- let us know if there's any way we can help @YangKeao !

@mkmik
Copy link

mkmik commented Sep 1, 2022

@kwiesmueller have you tried out https://github.com/mkmik/heappy ?

@caiobegotti
Copy link

Hi there @YangKeao do you still have time or are you still after this, please? That would be much appreciated by many people, given the time since the last update in the PR 🙇 cc: @mkmik too, I guess?

@mkmik
Copy link

mkmik commented Oct 6, 2023

@caiobegotti have you tried heappy? I would love some feedback

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.

is there any plan to support heap alloc analysis?
6 participants