Skip to content

Commit

Permalink
Fix bug generating flame graphs with deep stacks
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
godlygeek committed Jul 25, 2022
1 parent 2ef7339 commit c1ad550
Showing 1 changed file with 5 additions and 8 deletions.
13 changes: 5 additions & 8 deletions src/memray/reporters/flamegraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@

MAX_STACKS = int(sys.getrecursionlimit() // 2.5)

MAX_STACKS_NODE = {
"name": "<STACK TOO DEEP>",
"location": ["...", "...", 0],
"children": {},
}


def with_converted_children_dict(node: Dict[str, Any]) -> Dict[str, Any]:
stack = [node]
Expand Down Expand Up @@ -96,8 +90,10 @@ def from_snapshot(
if native_traces
else record.stack_trace()
)
num_skipped_frames = 0
for index, stack_frame in enumerate(reversed(stack)):
if is_cpython_internal(stack_frame):
num_skipped_frames += 1
continue
if (stack_frame, thread_id) not in current_frame["children"]:
node = create_framegraph_node_from_stack_frame(stack_frame)
Expand All @@ -109,8 +105,9 @@ def from_snapshot(
current_frame["thread_id"] = thread_id
unique_threads.add(thread_id)

if index > MAX_STACKS:
current_frame.update(MAX_STACKS_NODE)
if index - num_skipped_frames > MAX_STACKS:
current_frame["name"] = "<STACK TOO DEEP>"
current_frame["location"] = ["...", "...", 0]
break

transformed_data = with_converted_children_dict(data)
Expand Down

0 comments on commit c1ad550

Please sign in to comment.