From c1ad5506c56b27980624e55d70264f4c6bbbaeec Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Fri, 22 Jul 2022 18:41:21 -0400 Subject: [PATCH] Fix bug generating flame graphs with deep stacks When a stack becomes too deep, the flame graph reporter cuts off the stack and just displays a node with "" 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 --- src/memray/reporters/flamegraph.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/memray/reporters/flamegraph.py b/src/memray/reporters/flamegraph.py index a9330ec6e8..36c22d3cf2 100644 --- a/src/memray/reporters/flamegraph.py +++ b/src/memray/reporters/flamegraph.py @@ -17,12 +17,6 @@ MAX_STACKS = int(sys.getrecursionlimit() // 2.5) -MAX_STACKS_NODE = { - "name": "", - "location": ["...", "...", 0], - "children": {}, -} - def with_converted_children_dict(node: Dict[str, Any]) -> Dict[str, Any]: stack = [node] @@ -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) @@ -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"] = "" + current_frame["location"] = ["...", "...", 0] break transformed_data = with_converted_children_dict(data)