From 73fc932a304c711f5eb8fc80c51f6e41094bb8e1 Mon Sep 17 00:00:00 2001 From: Adnan Ali Date: Fri, 12 May 2017 10:29:22 -0400 Subject: [PATCH 1/3] fixing test for nodes. In the previous case, 4 is rendered as a child of 1. Except that 4 has a width of 2 and 1 has a width of 1. The child shouldn't be wider than the parent. The correct case is that there should be two nodes for 4 of width 1 each. The first node is a child of 1 and the second one is a child of 5. --- Gemfile | 3 ++- test/test_renderer.rb | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 5bb33d0..a000450 100644 --- a/Gemfile +++ b/Gemfile @@ -5,4 +5,5 @@ gemspec # both are optional, depending on platform gem 'fast_stack' -gem 'stackprof', platform: :mri_21 +gem 'stackprof', platform: [:mri_21, :mri_22, :mri_23] + diff --git a/test/test_renderer.rb b/test/test_renderer.rb index b419973..e8e5ec2 100644 --- a/test/test_renderer.rb +++ b/test/test_renderer.rb @@ -10,8 +10,9 @@ def test_builds_table_correctly {:x => 1, :y => 1, :frame => "1", :width => 2}, {:x => 1, :y => 2, :frame => "2", :width => 1}, {:x => 1, :y => 3, :frame => "3", :width => 1}, - {:x => 2, :y => 2, :frame => "4", :width => 2}, - {:x => 3, :y => 1, :frame => "5", :width => 1} + {:x => 2, :y => 2, :frame => "4", :width => 1}, + {:x => 3, :y => 1, :frame => "5", :width => 1}, + {:x => 3, :y => 2, :frame => "4", :width => 1} ], g.graph_data) end @@ -25,8 +26,9 @@ def test_avoids_bridges {:x => 1, :y => 1, :frame => "1", :width => 2}, {:x => 1, :y => 2, :frame => "2", :width => 1}, {:x => 1, :y => 3, :frame => "3", :width => 1}, - {:x => 2, :y => 2, :frame => "4", :width => 2}, - {:x => 3, :y => 1, :frame => "5", :width => 1} + {:x => 2, :y => 2, :frame => "4", :width => 1}, + {:x => 3, :y => 1, :frame => "5", :width => 1}, + {:x => 3, :y => 2, :frame => "4", :width => 1} ], g.graph_data) From 769a88d30a0c3bc7ea768dccf755ab47afe43ad5 Mon Sep 17 00:00:00 2001 From: Adnan Ali Date: Fri, 12 May 2017 10:45:37 -0400 Subject: [PATCH 2/3] fixing the code for frames with two parents. --- lib/flamegraph/renderer.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/flamegraph/renderer.rb b/lib/flamegraph/renderer.rb index 3d6dffe..cdab93a 100644 --- a/lib/flamegraph/renderer.rb +++ b/lib/flamegraph/renderer.rb @@ -24,6 +24,7 @@ def graph_html(embed_resources) def graph_data table = [] prev = [] + prev_parent = [] # a 2d array makes collapsing easy @stacks.each_with_index do |stack, pos| @@ -32,11 +33,16 @@ def graph_data col = [] - stack.reverse.map{|r| r.to_s}.each_with_index do |frame, i| + reversed_stack = stack.reverse + reversed_stack.map{|r| r.to_s}.each_with_index do |frame, i| + parent_frame = i > 0 ? reversed_stack[i - 1] : nil if !prev[i].nil? last_col = prev[i] - if last_col[0] == frame + frame_match = last_col[0] == frame + parent_match = parent_frame.nil? || prev_parent[i].nil? || parent_frame == prev_parent[i] + + if frame_match && parent_match last_col[1] += 1 col << nil next @@ -44,6 +50,7 @@ def graph_data end prev[i] = [frame, 1] + prev_parent[i] = parent_frame col << prev[i] end prev = prev[0..col.length-1].to_a From 53d55f4c701223205b9536df004c73d9946f3aec Mon Sep 17 00:00:00 2001 From: Adnan Ali Date: Fri, 12 May 2017 10:55:32 -0400 Subject: [PATCH 3/3] fixing additional case where the grandparents are different but the parents are shared. --- lib/flamegraph/renderer.rb | 5 ++++- test/test_renderer.rb | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/flamegraph/renderer.rb b/lib/flamegraph/renderer.rb index cdab93a..5ca4c69 100644 --- a/lib/flamegraph/renderer.rb +++ b/lib/flamegraph/renderer.rb @@ -32,12 +32,13 @@ def graph_data next unless stack col = [] + new_col = false reversed_stack = stack.reverse reversed_stack.map{|r| r.to_s}.each_with_index do |frame, i| parent_frame = i > 0 ? reversed_stack[i - 1] : nil - if !prev[i].nil? + if !prev[i].nil? && !new_col last_col = prev[i] frame_match = last_col[0] == frame parent_match = parent_frame.nil? || prev_parent[i].nil? || parent_frame == prev_parent[i] @@ -46,6 +47,8 @@ def graph_data last_col[1] += 1 col << nil next + else + new_col = true end end diff --git a/test/test_renderer.rb b/test/test_renderer.rb index e8e5ec2..b772d55 100644 --- a/test/test_renderer.rb +++ b/test/test_renderer.rb @@ -17,6 +17,21 @@ def test_builds_table_correctly end + def test_builds_table_correctly_for_different_grandparents + stacks = [["method4","method3","method1"],["method4","method3","method1"],["method4","method3","method2"]] + + g = Flamegraph::Renderer.new(stacks) + assert_equal([ + {:x => 1, :y => 1, :frame => "method1", :width => 2}, + {:x => 1, :y => 2, :frame => "method3", :width => 2}, + {:x => 1, :y => 3, :frame => "method4", :width => 2}, + {:x => 3, :y => 1, :frame => "method2", :width => 1}, + {:x => 3, :y => 2, :frame => "method3", :width => 1}, + {:x => 3, :y => 3, :frame => "method4", :width => 1} + ], g.graph_data) + + end + def test_avoids_bridges stacks = [["3","2","1"],["4","1"],["4","5"]]