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

Fix bug generating flame graphs with deep stacks #164

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

godlygeek
Copy link
Contributor

@godlygeek godlygeek commented Jul 22, 2022

When a stack becomes too deep, the flame graph reporter cuts off the
stack and just displays a node with <STACK TOO DEEP> as the name. Up
until now, all such nodes would share the same dict of child nodes. This
was usually fine, because we should never attempt to add child nodes
below a node that was already too deep. However, subtly, if there were
several different call stacks that led to the too-deep node, and they
differed only in ignored CPython-internal frames, and they had
a different number of ignored frames, then it was possible that the same
node in our tree, when reached by two different paths, would be found at
two different depths. If a later call found the same node at a shallower
depth, we would try to add children to it, modifying the children of all
of the too-deep nodes.

Fix both of these problems. Stop assigning the same dict to every
too-deep node, and instead just use the empty dict of children that it
already holds. Also, stop counting skipped frames towards our depth
limit. This removes any risk of us adding nodes below a node that was
already too deep in the future, and also ensures that all of the marker
nodes for a truncated stack appear at the same depth in the flame graph.

Closes: #163

When a stack becomes too deep, the flame graph reporter cuts off the
stack and just displays a node with "<STACK TOO DEEP>" as the name. Up
until now, all such nodes would share the same dict of child nodes. This
was usually fine, because we should never attempt to add child nodes
below a node that was already too deep. However, subtly, if there were
several different call stacks that led to the too-deep node, and they
differed only in ignored CPython-internal frames, and they had
a different number of ignored frames, then it was possible that the same
node in our tree, when reached by two different paths, would be found at
two different depths. If a later call found the same node at a shallower
depth, we would try to add children to it, modifying the children of all
of the too-deep nodes.

Fix both of these problems. Stop assigning the same dict to every
too-deep node, and instead just use the empty dict of children that it
already holds. Also, stop counting skipped frames towards our depth
limit. This removes any risk of us adding nodes below a node that was
already too deep in the future, and also ensures that all of the marker
nodes for a truncated stack appear at the same depth in the flame graph.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek godlygeek merged commit c1ad550 into bloomberg:main Jul 25, 2022
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.

Slow run and error generating flamegraph
2 participants