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

Version 0.2 Release Candidate #42

Merged
merged 52 commits into from
Nov 30, 2014
Merged

Version 0.2 Release Candidate #42

merged 52 commits into from
Nov 30, 2014

Conversation

jiffyclub
Copy link
Owner

This is a pretty big rewrite of SnakeViz mostly with the goal of improving performance. I've thrown a large and detailed profile at this version and it seems to work just fine, so I'm hoping this will be eliminate the performance issues we all had with version 0.1. Please take this for a spin and leave any feedback here.

The big, high level changes are:

  • Do minimal pre-processing of stats in Python, then embed the data in the HTML for use by the client code
  • Do all of the function call-tree building in the client
  • Limit the depth of the displayed call tree (depth defaults to 10)
  • Filter out arcs that would be too small to see (reduces the number of elements in the SVG)

Other, smaller changes include:

  • On-demand view of the current call stack (the stack to the current center of the sunburst)
  • No more tooltips, function info is displayed to the side of the sunburst

Screenshot of the new stuff in action:

screen shot 2014-11-20 at 11 40 17 am

The call stack collapses so it won't always cover the sunburst.

So far: Right now it'll make a visualization with highlighting
and a tooltip, but no zooming.

snakeviz.js:

  - function for finding the root of the function call stack
  - function for building a JSON tree for use with D3's partition
    - still needs to handle recursion
Problems:

- Sizes are wrong. This is because I'm calculating the size based
  on the total cumulative time for a function, not the cumulative time
  below a given function.
- Need to remove directories from file names.
There are times when children have calculated sizes larger than
their parent (because with the profiling information we have it is
hard to untangle just how much cumulative time of a program was spent
in a given tree). When this happens I don't do anything fancy,
only normalize the children's sizes so they exactly fit within the parent.
- Internal-time children larger than their parents
- Random floating arcs
- Tool-tip text overflowing the tooltip brackground
Call stack does not update when open and user clicks on vis.
Instead of having a tooltip displaying function information,
have a div of info on the left side showing stuff.
Gives more room to display info.
@jiffyclub
Copy link
Owner Author

@tonysyu: Thanks for the link, I'll check that out.

@kburns: Would it be possible to share one of your huge profiles with me by email? It seems like nothing I have on hand is stressing this out enough. I'm jiffyclub@gmail.com. Thanks!

@kburns
Copy link

kburns commented Nov 24, 2014

Sure thing

I was calculating child sizes before recuring into the callstack,
but only normalizing them afterwards. That means that children
could be added to children with grossly wrong sizes and they would
blow up their parents' display sizes.

So now I calculate and normalize all child sizes before recursing
further into the call stack so that no part of the tree can be larger
than its parent.
@jiffyclub
Copy link
Owner Author

@tonysyu, I think your issue should be fixed by 8793568. Nodes deep in the call stack with larger cumulative times were inappropriately blowing up the sizes of their parents. Thanks for catching that.

@tonysyu
Copy link

tonysyu commented Nov 25, 2014

That was a quick turnaround! This is a great v2 release 👍

Building some call trees for the JSON worker can take a while and it
leaves the page unresponsive while the user could be browsing the
stats table or changing parameters. Moving that processing to a
Worker allows the page to stay responsive so that users can change
visualization parameters or browse the stats table.
Spinner is visible whie the app is building a call tree and
rendering the SVG.
The fraction of time spent in a child of a function is compared to the cutoff
and if it is less than the cutoff the child is added to the call tree
but none of its children are.

The default threshold is 1 / 1000, with options for 1 / 100 and no cutoff.
Currently the worker can fail when the call tree it tries to send
back is so large that it can't be serialized to a JSON string.
Now the app shows an error message with some advice when that
happens.
@jiffyclub
Copy link
Owner Author

@kburns, thanks for sending over the profile, that was really helpful. You've got what you might call a "wide" profile, with a lot of calls off of the root of the call tree. That leads to a large call tree even when depth-limited, and that was bogging down the app.

I've added some features for mitigating this situation:

  • Changed the initial depth limit from 10 to 5 (and added 3 as an option)
  • Moved the building of the call stack to a Worker thread so the app remains responsive while the call tree is being constructed. This means you can still use the stats table while gears are turning. The app still locks up when the SVG is being rendered, though.
  • Added a "cutoff" option. This causes the app to skip descending into parts of the call tree that take up less than some fraction of the cumulative time of their parent, leading to a smaller call tree. The cutoff defaults to 1/1000, with options for 1/100 and no cutoff.
  • The app now caches call trees in memory so when you revisit functions you've clicked on before less work is done. (Again, rendering the SVG can still take noticeable time.)
  • Added a little spinner so you can see when the app is working.

Now your profile renders for me in a few seconds and I can speed things up by decreasing the depth and increasing the cutoff. Thanks again for providing that use case!

jiffyclub added a commit that referenced this pull request Nov 30, 2014
Rewrite for version 0.2.
@jiffyclub jiffyclub merged commit f66c522 into master Nov 30, 2014
@jiffyclub jiffyclub deleted the v2 branch November 30, 2014 22:29
@jiffyclub
Copy link
Owner Author

Thanks for your help @kburns and @tonysyu, new release should be out shortly!

@kburns
Copy link

kburns commented Nov 30, 2014

Thank you! The new version is working great for me!

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

3 participants