Filtering, Zooming, Detailed Analysis

[thisduck]

Hi Sam,

This is a pull request for the changes I mentioned in #23

Here is a list of changes:

1. Zooming is based on clicking on a node.
2. You can zoom out by one level by pressing the (-) dash/minus button on the keyboard. You reset the zoom entirely by pressing (0) zero on the keyboard (or clicking the reset zoom button).
3. There is a detailed breakdown of the information of a node on hovering.
4. The gems listing has been changed to display both gems and methods.
5. The listings are sorted by "self time," the time the method spends in itself excluding time spent in any children methods.
6. As you zoom in or out, the gem and methods listings display data from the visible areas.
7. Filtering can be done by an include and exclude filter list. The filter searches for any text in the frame. So you can filter by file path, file name, method name, class name, etc.
8. The gems and methods listing changes based on the filtered data (along with the zoomed data).
9. The backtrace modal (triggered by a right click on a node) displays detailed information about the frame, along with a tab that shows any direct method call by the current method.

This is a fairly large rewrite (it was the only way to do it to add filtering and zooming in this way and maintain speed).

I'd love it if you were able to give this a test run on your own code and share the experience.

[thisduck]

I plan on adding a few more features that will make the gem/method listing areas even more useful.

[SamSaffron]

wow, awesome can you do some screenshots

[SamSaffron]

this is a bit of a problem, I like that we still have the options to completely embed flamegraph behind a firewall, I don't want to have assets that import stuff externally that is not checked in to the repo like the font here.

[thisduck]

@SamSaffron Got it. Will fix.

[thisduck]

@SamSaffron I ended up removing the font from the file, since it defaults to Helvetica. I did add in base64 for the icon fonts.

[SamSaffron]

in particular the font is missing here

[thisduck]

@SamSaffron I'll do some screenshots soon.

I also want to build in some keyboard based shortcuts and controls, to make investigative navigation easier.

[thisduck]

@SamSaffron I can also do a video tutorial as well. On strategies to approach and navigate performance issues with this interface. Even as I'm using it now, I'm finding that I want features just to make it easier and simpler.

I kinda want this to be really fun and easy to use, that's the drive anyway.

[SamSaffron]

Very cool, very happy you are working on this!

…

On Sun, Apr 30, 2017 at 10:56 PM, Adnan Ali ***@***.***> wrote: @SamSaffron <https://github.com/SamSaffron> I can also do a video tutorial as well. On strategies to approach and navigate performance issues with this interface. Even as I'm using it now, I'm finding that I want features just to make it easier and simpler. I kinda want this to be really fun and easy to use, that's the drive anyway. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAUXQZcil1lQpPAWevZ0e3KN5bWKqT-ks5r1Un7gaJpZM4NMah8> .

[thisduck]

Passing opts here because I added javascript based filters. So that the page can have default filters as it loads up.

[SamSaffron]

My biggest blocker here on merging is that I find it super hard to navigate.

Can mousewheel support be added back in? and click and drag graph around support?

[thisduck]

@SamSaffron One of the reasons I changed it to click to zoom is because I had found click and drag harder to navigate, specially on larger datasets. And had similar feedback from others who tried the gem but ended up not using it.

I had a similar experience a few years ago, too. Where speed and navigation prevented from using the gem.

But I respect that existing users have a certain way and comfort of navigating. I do feel like click to zoom is easier overall once you're used to it. Specially to newer users.

[thisduck]

@SamSaffron Also, I changed the nodes to be html based in order to get the text in there, without compromising on speed.

To get click and drag zoom to be nice, I'd have to change back to SVG nodes. But then that would make text nodes slow.

My intent is to improve speed as that impacts usage.

[thisduck]

@SamSaffron Is there something we can do halfway?

Just been thinking about this. I see the benefit, of course, of being able to click and drag. It's just the speed concerns.

Let me process this and see if something comes to mind.

[SamSaffron]

@thisduck any chance we can merge this in as an "option" keep the existing implementation as 1 options and add yours as a second option, they whoever consumes flamegraph chooses the rendering option and you can control and manage (2) completely ?

[thisduck]

@SamSaffron Sounds good. Let me see what I can do in the coming weeks. Thanks!

[thisduck]

Hey @SamSaffron,

One of the reasons I wasn't too keen on retaining the zooming functionality was because with a lot of nodes, it got super slow and hard to maintain. But I've been reading about D3 and Canvas recently and there's ways we can make everyone happy and get all the filtering and analysis goodness. We can potentially have one unified UI this way.

I want to try and refactor the code as well so that it's easier to read and contribute to. How do you feel about a phased approach to getting there? (Something like updating the current code base to ES6, using classes/objects, etc)

I have a fairly decent idea of how to get a bunch of the functionality in there in a performant way since its' already implemented in this PR. But there's a lot we can clean up. And I'll have some time on my hands over December.

(btw, in this PR, I had recently added the ability to track SQL queries through active record, making N+1 detection at the query level clear in the UI!!)

[thisduck]

@SamSaffron

I've started to create smaller PRs on my fork: https://github.com/thisduck/flamegraph/pulls

This way the changes can be small and merged in bit by bit into master.

[SamSaffron]

Awesome, feel free to submit the PRs here, as long as there is toggle between new mode and old mode I am perfectly happy with stuff landing even now.

…

On Sat, Dec 1, 2018 at 1:49 AM Adnan Ali ***@***.***> wrote: @SamSaffron <https://github.com/SamSaffron> I've started to create smaller PRs on my fork: https://github.com/thisduck/flamegraph/pulls This way the changes can be small and merged in bit by bit into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAUXUVlQd9gvBDUyua4htoyeTxKvzldks5u0UVggaJpZM4NMah8> .

[thisduck]

@SamSaffron Have been plugging away and am very close to something sharable soon.

When you get a moment, can you please send me an example of a flamegraph html that you've been looking at recently? (A few if possible, but one will do great!) Thanks!

[SamSaffron]

Hi @thisduck,

Attached is one I looked at today. I picked this issue up: https://review.discourse.org/t/perf-automatic-upload-size-calculation-not-persisted/868

homepage.zip

[jaydorsey]

I am super late to this, but I just took a look at the branch @thisduck created and overall I'm a fan.

• I love how it's zoomed to 100% of the width of the window
• It's much more responsive to zoom in exactly where I want (once I get used to the controls)
• I had to add newrelic_apm as a dependency in my app (we don't use it)
• This graph, on the same codebase/endpoint, seemed to display the postgres method calls where i expected them to here

I do like the mouse zoom in the old one but it was hit or miss performance wise depending on the graph. Even on a well-equipped Macbook Pro zooming in and what not could be difficult

It would be great to be able to toggle between the two

[thisduck]

Thanks @jaydorsey. I'm going to be without a job in a couple of weeks and can give this attention so that there's either a toggle or some integrated version of the interfaces.

The newrelic_apm dependency was there to display SQL queries and greater N + 1 visibility and that can be made optional. I made it only work with mysql, so it's very limited at the moment.

@SamSaffron Any thoughts if I were to use something like an Ember or React for the interface?

[SamSaffron]

@thisduck sorry to hear you are out of work, wishing you lots of luck with your job hunt. I am fine with ember or react for an interface here.

[thisduck]

So my timeline to work on this wasn't accurate. But am keen on doing something now.

@jaydorsey Would you be able to share the html of some flamegraphs that you were dealing with?
Particularly ones that were slow?

[jaydorsey]

@jaydorsey Would you be able to share the html of some flamegraphs that you were dealing with?
Particularly ones that were slow?

I wasn't able to find one of the larger graphs since you'd asked, but since then I've moved on to a different role so I won't be able to reproduce it exactly. I think i can fake one though (similar-ish behavior) with a small demo app if that would be useful.