Skip to content

Commit

Permalink
flamegraph: Remove the flamegraph feature
Browse files Browse the repository at this point in the history 
This unfortunately never worked correctly due to a fundamental
limitation with the XHProf data format, which is that it only
records metadata per parent-child method combination, it cannot
be used to build a complete call tree.

The feature was added in pull perftools#177. More information about this
limitation is described in detail at perftools#219, perftools#216 and perftools#212.

A brief summary:

* The visualisation showed callstacks that did not actually exist,
  and was missing callstacks that did exist. (Due to assuming
  that every combination of a parent-child pair is valid, and
  due to it randomly assinging repeated calls to the first
  encountered parent.)

* The visualisation discarded all timing values from XHProf,
  except for the timing of leaf nodes (methods without children),
  which were then added up recursively. The end result was a
  visually well-balanced tree, but with timing values that were
  not related to the actual performance (upto 100x inflated), and
  the proportions were incorrect as well, making some code look
  fast instead of slow, and vice versa.

These are inherent problems that cannot be solved because the
information logically required to make a flamegraph (call stacks)
is not collected by XHProf.

This closes perftools#216, perftools#212, perftools#211, perftools#207.
This fixes perftools#212.
  • Loading branch information
@Krinkle
Krinkle committed Sep 23, 2018
1 parent 3279066 commit 134f320
Show file tree
Hide file tree
Showing 10 changed files with 0 additions and 1,175 deletions.
25 changes: 0 additions & 25 deletions src/Xhgui/Controller/Run.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,31 +272,6 @@ public function callgraphData()
return $response->body(json_encode($callgraph));
}

public function flamegraph()
{
$request = $this->_app->request();
$profile = $this->_profiles->get($request->get('id'));

$this->_template = 'runs/flamegraph.twig';
$this->set(array(
'profile' => $profile,
'date_format' => $this->_app->config('date.format'),
));
}

public function flamegraphData()
{
$request = $this->_app->request();
$response = $this->_app->response();
$profile = $this->_profiles->get($request->get('id'));
$metric = $request->get('metric') ?: 'wt';
$threshold = (float)$request->get('threshold') ?: 0.01;
$flamegraph = $profile->getFlamegraph($metric, $threshold);

$response['Content-Type'] = 'application/json';
return $response->body(json_encode($flamegraph));
}

public function callgraphDataDot()
{
$request = $this->_app->request();
Expand Down
73 changes: 0 additions & 73 deletions src/Xhgui/Profile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,79 +574,6 @@ protected function _callgraphData($parentName, $main, $metric, $threshold, $pare
}
}

/**
* Return a structured array suitable for generating flamegraph visualizations.
*
* Functions whose inclusive time is less than 1% of the total time will
* be excluded from the callgraph data.
*
* @return array
*/
public function getFlamegraph($metric = 'wt', $threshold = 0.01)
{
$valid = array_merge($this->_keys, $this->_exclusiveKeys);
if (!in_array($metric, $valid)) {
throw new Exception("Unknown metric '$metric'. Cannot generate flamegraph.");
}
$this->calculateSelf();

// Non exclusive metrics are always main() because it is the root call scope.
if (in_array($metric, $this->_exclusiveKeys)) {
$main = $this->_maxValue($metric);
} else {
$main = $this->_collapsed['main()'][$metric];
}

$this->_visited = $this->_nodes = $this->_links = array();
$flamegraph = $this->_flamegraphData(self::NO_PARENT, $main, $metric, $threshold);
return array('data' => array_shift($flamegraph), 'sort' => $this->_visited);
}

protected function _flamegraphData($parentName, $main, $metric, $threshold, $parentIndex = null)
{
$result = array();
// Leaves don't have children, and don't have links/nodes to add.
if (!isset($this->_indexed[$parentName])) {
return $result;
}

$children = $this->_indexed[$parentName];
foreach ($children as $childName => $metrics) {
$metrics = $this->_collapsed[$childName];
if ($metrics[$metric] / $main <= $threshold) {
continue;
}
$current = array(
'name' => $childName,
'value' => $metrics[$metric],
);
$revisit = false;

// Keep track of which nodes we've visited and their position
// in the node list.
if (!isset($this->_visited[$childName])) {
$index = count($this->_nodes);
$this->_visited[$childName] = $index;
$this->_nodes[] = $current;
} else {
$revisit = true;
$index = $this->_visited[$childName];
}

// If the current function has more children,
// walk that call subgraph.
if (isset($this->_indexed[$childName]) && !$revisit) {
$grandChildren = $this->_flamegraphData($childName, $main, $metric, $threshold, $index);
if (!empty($grandChildren)) {
$current['children'] = $grandChildren;
}
}

$result[] = $current;
}
return $result;
}

public function toArray()
{
return $this->_data;
Expand Down
9 changes: 0 additions & 9 deletions src/routes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,6 @@
$di['runController']->callgraphData();
})->name('run.callgraph.data');

$app->get('/run/flamegraph', function () use ($di, $app) {
$app->controller = $di['runController'];
$app->controller->flamegraph();
})->name('run.flamegraph');

$app->get('/run/flamegraph/data', function () use ($di, $app) {
$di['runController']->flamegraphData();
})->name('run.flamegraph.data');

$app->get('/run/callgraph/dot', function () use ($di, $app) {
$di['runController']->callgraphDataDot();
})->name('run.callgraph.dot');
Expand Down
119 changes: 0 additions & 119 deletions src/templates/runs/flamegraph.twig
View file

This file was deleted.

3 changes: 0 additions & 3 deletions src/templates/runs/view.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@
<a href="{{ url('run.compare', {base: result.id|trim }) }}" id="compare-button" class="btn back-link">
Compare this run
</a>
<a href="{{ url('run.flamegraph', {id: result.id|trim }) }}" class="btn back-link">
View Flamegraph
</a>
<a href="{{ url('run.callgraph', {id: result.id|trim }) }}" class="btn back-link">
View Callgraph
</a>
Expand Down
120 changes: 0 additions & 120 deletions tests/ProfileTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,124 +490,4 @@ public function testGetDateFallback()
$result = $profile->getDate();
$this->assertInstanceOf('DateTime', $result);
}

public function testGetFlamegraphBasic()
{
$fixture = $this->_fixture[0];
$profile = new Xhgui_Profile($fixture);
$result = $profile->getFlamegraph();
$expected = array(
'data' => array(
'name' => 'main()',
'value' => 50139,
'children' => array(
array(
'name' => 'strpos()',
'value' => 600
)
),
),
'sort' => array(
'main()' => 0,
'strpos()' => 1
)
);
$this->assertEquals($expected, $result);
}

public function testGetFlamegraphNonUniqueLeaf()
{
$fixture = array(
'profile' => array(
'main()' => array(
'wt' => 10000,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'main()==>one()' => array(
'wt' => 2000,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'main()==>two()' => array(
'wt' => 4000,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'main()==>three()' => array(
'wt' => 4000,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'one()==>util()' => array(
'wt' => 1500,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'two()==>util()' => array(
'wt' => 3500,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'three()==>special()' => array(
'wt' => 3500,
'ct' => 1, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
'util()==>util_helper()' => array(
'wt' => 3900,
'ct' => 2, 'cpu' => 1, 'mu' => 1, 'pmu' => 1,
),
),
);
$profile = new Xhgui_Profile($fixture);
$result = $profile->getFlamegraph();
$expected = array(
'data' => array(
'name' => 'main()',
'value' => 10000,
'children' => array(
array(
'name' => 'one()',
'value' => 2000,
'children' => array(
array(
'name' => 'util()',
'value' => 5000,
'children' => array(
array(
'name' => 'util_helper()',
'value' => 3900,
),
),
),
),
),
array(
'name' => 'two()',
'value' => 4000,
'children' => array(
array(
'name' => 'util()',
'value' => 5000
),
),
),
array(
'name' => 'three()',
'value' => 4000,
'children' => array(
array(
'name' => 'special()',
'value' => 3500,
),
),
),
),
),
'sort' => array(
'main()' => 0,
'one()' => 1,
'util()' => 2,
'util_helper()' => 3,
'two()' => 4,
'three()' => 5,
'special()' => 6,
)
);
$this->assertEquals($expected, $result);
}
}
Loading

0 comments on commit 134f320

Please sign in to comment.