Add option for hidden stack frame alignment #22
Conversation
In 0x, hidden stack frames still take up space. That's visible as empty space between different stacks: one of the stacks is a child of a hidden stack frame. This is the easiest thing to do d3 wise and has the nice benefit that stack frames only move vertically when you toggle frame visibility. This PR adds a `collapseHiddenNodeWidths: true` option to change that behaviour. Hidden stack frames don't take up horizontal space when this option is set. That means that child stacks of hidden stack frames are bunched up next to each other without empty space in between. The benefit of this approach is that, combined with `heatBars: true`, the heat bars will be one contiguous block, and the size of this block is directly proportional to the time that frame was at the top of the stack. Without this option, heat bars are seen through all the gaps where hidden stack frames are taking up space, and only the colour is proportional to the heat. It's quite a mouthful for a fairly small code change...
| }) | ||
| .sort(doSort) | ||
|
|
||
| // Make "all stacks" as wide as every visible stack. | ||
| data.value = data.children ? data.children.reduce(sumChildValues, 0) : 0 | ||
| data.value = data.children | ||
| ? data.children.reduce((acc, node) => acc + node.value, 0) |
There was a problem hiding this comment.
sumChildValues only accidentally worked here, here we're dealing with d3 node wrappers whereas sumChildValues expects 0x-style node data
There was a problem hiding this comment.
LGTM in terms of feature and functionality.
Just one nit in the readme, and general comment that I don't think this is as niche or edge-case-y an option as you think - our specific reason for needing it now is, but in general it's pretty valid / understandable that some flamegraph users might want there to not be gaps left over from things that are hidden.
Possibly … I guess I just needed a lot of words to make it clear to myself, so I thought it'd maybe be quite difficult to understand for others! |
|
LGTM |
This is a bit tough to explain so I've got words here and images down below 👇
In 0x, hidden stack frames still take up space. That's visible as empty
space between different stacks: one of the stacks is a child of a hidden
stack frame. This is the easiest thing to do d3 wise and has the nice
benefit that stack frames only move vertically when you toggle frame
visibility.
This PR adds a
collapseHiddenNodeWidths: trueoption to change thatbehaviour. Hidden stack frames don't take up horizontal space when this
option is set. That means that child stacks of hidden stack frames are
bunched up next to each other without empty space in between. The
benefit of this approach is that, combined with
heatBars: true, theheat bars will be one contiguous block, and the size of this block is
directly proportional to the time that frame was at the top of the stack.
Without this option, heat bars are seen through all the gaps where
hidden stack frames are taking up space, and only the colour is
proportional to the heat.
Visually:
collapseHiddenNodeWidths: false
Here, the
_GI__frame is a C++ frame. It's a child of a V8 frame. When the V8 frames are hidden, the C++ frame is aligned to the left of the V8 frame it's in, because the other hidden V8 frame widths still push it to the right.collapseHiddenNodeWidths: true
With this option enabled, the C++ frame is aligned to the left of the closest visible frame, rather than its direct parent.
This also fixes an unrelated issue that got in the way while testing this, where very narrow stack frames were not drawn at all. Now, very narrow frames are drawn at 1px width.