diff --git a/lib/tests/xhprof_test.php b/lib/tests/xhprof_test.php index dac3f2168be..60ccbf910db 100644 --- a/lib/tests/xhprof_test.php +++ b/lib/tests/xhprof_test.php @@ -26,12 +26,17 @@ namespace core; */ class xhprof_test extends \advanced_testcase { + public static function setUpBeforeClass(): void { + global $CFG; + require_once($CFG->libdir . '/xhprof/xhprof_moodle.php'); + } + /** * Data provider for string matches * * @return array */ - public function profiling_string_matches_provider() { + public static function profiling_string_matches_provider(): array { return [ ['/index.php', '/index.php', true], ['/some/dir/index.php', '/index.php', false], @@ -61,19 +66,180 @@ class xhprof_test extends \advanced_testcase { /** * Test the matching syntax * + * @covers ::profiling_string_matches * @dataProvider profiling_string_matches_provider * @param string $string * @param string $patterns * @param bool $expected */ public function test_profiling_string_matches($string, $patterns, $expected) { - - global $CFG; - require_once($CFG->libdir . '/xhprof/xhprof_moodle.php'); - $result = profiling_string_matches($string, $patterns); $this->assertSame($result, $expected); } + /** + * Data provider for both the topological sort and the data reduction tests. + * + * @return array + */ + public static function run_data_provider(): array { + // This data corresponds to the runs used as example @ MDL-79285. + return [ + 'sorted_case' => [ + 'rundata' => array_flip([ + 'A', + 'A==>B', + 'A==>C', + 'A==>__Mustache4', + 'B==>__Mustache1', + '__Mustache1==>__Mustache2', + '__Mustache4==>__Mustache2', + '__Mustache4==>E', + 'E==>F', + 'C==>F', + '__Mustache2==>F', + '__Mustache2==>D', + 'D==>__Mustache3', + '__Mustache3==>F', + ]), + 'expectations' => [ + 'topofirst' => 'A', + 'topolast' => '__Mustache3==>F', + 'topocount' => 14, + 'topoorder' => [ + // Before and after pairs to verify they are ordered. + ['before' => 'A==>C', 'after' => 'C==>F'], + ['before' => 'D==>__Mustache3', 'after' => '__Mustache3==>F'], + ], + 'reducecount' => 8, + 'reduceremoved' => [ + // Elements that will be removed by the reduction. + '__Mustache1==>__Mustache2', + '__Mustache4==>__Mustache2', + '__Mustache2==>F', + '__Mustache2==>D', + '__Mustache2==>D', + '__Mustache3==>F', + ], + ], + ], + 'unsorted_case' => [ + 'rundata' => array_flip([ + 'A==>__Mustache4', + '__Mustache3==>F', + 'A==>B', + 'A==>C', + 'B==>__Mustache1', + '__Mustache1==>__Mustache2', + '__Mustache4==>__Mustache2', + '__Mustache4==>E', + 'E==>F', + 'C==>F', + '__Mustache2==>F', + '__Mustache2==>D', + 'D==>__Mustache3', + 'A', + ]), + 'expectations' => [ + 'topofirst' => 'A', + 'topolast' => '__Mustache3==>F', + 'topocount' => 14, + 'topoorder' => [ + // Before and after pairs to verify they are ordered. + ['before' => 'A==>C', 'after' => 'C==>F'], + ['before' => 'D==>__Mustache3', 'after' => '__Mustache3==>F'], + ], + 'reducecount' => 8, + 'reduceremoved' => [ + // Elements that will be removed by the reduction. + '__Mustache1==>__Mustache2', + '__Mustache4==>__Mustache2', + '__Mustache2==>F', + '__Mustache2==>D', + '__Mustache2==>D', + '__Mustache3==>F', + ], + ], + ], + ]; + } + + /** + * Test that topologically sorting the run data works as expected + * + * @covers \moodle_xhprofrun::xhprof_topo_sort + * @dataProvider run_data_provider + * + * @param array $rundata The run data to be sorted. + * @param array $expectations The expected results. + */ + public function test_xhprof_topo_sort(array $rundata, array $expectations) { + // Make sure all the examples in the provider are the same size. + $this->assertSame($expectations['topocount'], count($rundata)); + + // Make moodle_xhprofrun::xhprof_topo_sort() accessible. + $reflection = new \ReflectionClass('\moodle_xhprofrun'); + $method = $reflection->getMethod('xhprof_topo_sort'); + $method->setAccessible(true); + // Sort the data. + $result = $method->invokeArgs(new \moodle_xhprofrun(), [$rundata]); + $this->assertIsArray($result); + $this->assertSame($expectations['topocount'], count($result)); + // Convert the array to a list of keys, so we can assert values by position. + $resultkeys = array_keys($result); + + // This is the elements that should be first. + $this->assertSame($expectations['topofirst'], $resultkeys[0]); + // This is the element that should be last. + $this->assertSame($expectations['topolast'], $resultkeys[$expectations['topocount'] - 1]); + // This relative ordering should be respected. + foreach ($expectations['topoorder'] as $order) { + // All the elements in the expectations should be present. + $this->assertArrayHasKey($order['before'], $result); + $this->assertArrayHasKey($order['after'], $result); + // And they should be in the correct relative order. + $this->assertGreaterThan( + array_search($order['before'], $resultkeys), + array_search($order['after'], $resultkeys) + ); + } + + // Final check, if we sort it again, nothing changes (it's already topologically sorted). + $result2 = $method->invokeArgs(new \moodle_xhprofrun(), [$result]); + $this->assertSame($result, $result2); + } + + /** + * Test that reducing the data complexity works as expected + * + * @covers \moodle_xhprofrun::reduce_run_data + * @dataProvider run_data_provider + * + * @param array $rundata The run data to be reduced. + * @param array $expectations The expected results. + */ + public function test_reduce_run_data(array $rundata, array $expectations) { + // Make sure that the expected keys that will be removed are present. + foreach ($expectations['reduceremoved'] as $key) { + $this->assertArrayHasKey($key, $rundata); + } + + // Make moodle_xhprofrun::reduce_run_data() accessible. + $reflection = new \ReflectionClass('\moodle_xhprofrun'); + $method = $reflection->getMethod('reduce_run_data'); + $method->setAccessible(true); + // Reduce the data. + $result = $method->invokeArgs(new \moodle_xhprofrun(), [$rundata]); + $this->assertIsArray($result); + $this->assertSame($expectations['reducecount'], count($result)); + // These have been the removed elements. + foreach ($expectations['reduceremoved'] as $key) { + $this->assertArrayNotHasKey($key, $result); + } + + // Final check, if we reduce it again, nothing changes (it's already reduced). + $result2 = $method->invokeArgs(new \moodle_xhprofrun(), [$result]); + $this->assertSame($result, $result2); + } } diff --git a/lib/xhprof/xhprof_html/callgraph.php b/lib/xhprof/xhprof_html/callgraph.php index 5076eab9ef5..a1415341974 100644 --- a/lib/xhprof/xhprof_html/callgraph.php +++ b/lib/xhprof/xhprof_html/callgraph.php @@ -90,6 +90,7 @@ if (!array_key_exists($type, $xhprof_legal_image_types)) { // Start moodle modification: use own XHProfRuns implementation. // $xhprof_runs_impl = new XHProfRuns_Default(); $xhprof_runs_impl = new moodle_xhprofrun(); +$xhprof_runs_impl->set_reducedata(xhprof_get_bool_param('reducedata', 1)); // Reduce data by default. // End moodle modification. if (!empty($run)) { diff --git a/lib/xhprof/xhprof_html/index.php b/lib/xhprof/xhprof_html/index.php index 80e7821fe92..4d29234ee7f 100644 --- a/lib/xhprof/xhprof_html/index.php +++ b/lib/xhprof/xhprof_html/index.php @@ -92,6 +92,12 @@ $vgbar = ' class="vgbar"'; // Start moodle modification: use own XHProfRuns implementation. // $xhprof_runs_impl = new XHProfRuns_Default(); $xhprof_runs_impl = new moodle_xhprofrun(); +$reducedata = xhprof_get_bool_param('reducedata', 0); // Don't reduce data by default. +$xhprof_runs_impl->set_reducedata($reducedata); +if ($reducedata) { + // We need to inject it, so we continue in "reduced data mode" all the time. + $params['reducedata'] = $reducedata; +} // End moodle modification. displayXHProfReport($xhprof_runs_impl, $params, $source, $run, $wts, diff --git a/lib/xhprof/xhprof_moodle.php b/lib/xhprof/xhprof_moodle.php index b87916dbdd2..86c63a6d526 100644 --- a/lib/xhprof/xhprof_moodle.php +++ b/lib/xhprof/xhprof_moodle.php @@ -859,6 +859,9 @@ class moodle_xhprofrun implements iXHProfRuns { protected $totalmemory = 0; protected $timecreated = 0; + /** @var bool Decide if we want to reduce profiling data or no */ + protected bool $reducedata = false; + public function __construct() { $this->timecreated = time(); } @@ -888,7 +891,15 @@ class moodle_xhprofrun implements iXHProfRuns { if (@gzuncompress(base64_decode($rec->data)) === false) { return unserialize(base64_decode($rec->data)); } else { - return unserialize(gzuncompress(base64_decode($rec->data))); + $info = unserialize(gzuncompress(base64_decode($rec->data))); + if (!$this->reducedata) { + // We want to return the full data. + return $info; + } + + // We want to apply some transformations here, in order to reduce + // the information for some complex (too many levels) cases. + return $this->reduce_run_data($info); } } @@ -963,11 +974,151 @@ class moodle_xhprofrun implements iXHProfRuns { $this->url = $url; } - // Private API starts here + /** + * Enable or disable reducing profiling data. + * + * @param bool $reducedata Decide if we want to reduce profiling data (true) or no (false). + */ + public function set_reducedata(bool $reducedata): void { + $this->reducedata = $reducedata; + } + + // Private API starts here. protected function sum_calls($sum, $data) { return $sum + $data['ct']; } + + /** + * Reduce the run data to a more manageable size. + * + * This removes from the run data all the entries that + * are matching a group of regular expressions. + * + * The main use is to remove all the calls between "__Mustache" + * functions, which don't provide any useful information and + * make the call-graph too complex to be handled. + * + * @param array $info The xhprof run data, original array. + * @return array The xhprof run data, reduced array. + */ + protected function reduce_run_data(array $info): array { + // Define which (regular expressions) we want to remove. Already escaped if needed to, please. + $toremove = [ + '__Mustache.*==>__Mustache.*', // All __Mustache to __Mustache calls. + ]; + // Build the regular expression to be used. + $regexp = '/^(' . implode('|', $toremove) . ')$/'; + + // Given that the keys of the array have the format "parent==>child" + // we want to rebuild the array with the same structure but + // topologically sorted (parents always before children). + // Note that we do this exclusively to guarantee that the + // second pass (see below) works properly in all cases because, + // without it, we may need to perform N (while loop) second passes. + $sorted = $this->xhprof_topo_sort($info); + + // To keep track of removed and remaining (child-parent) pairs. + $removed = []; + $remaining = []; + + // First pass, we are going to remove all the elements which + // both parent and child are __Mustache function calls. + foreach ($sorted as $key => $value) { + if (!str_contains($key, '==>')) { + $parent = 'NULL'; + $child = $key; + } else { + [$parent, $child] = explode('==>', $key); // TODO: Consider caching this in a property. + } + + if (preg_match($regexp, $key)) { + unset($sorted[$key]); + $removed[$child][$parent] = true; + } else { + $remaining[$child][$parent] = true; + } + } + + // Second pass, we are going to remove all the elements which + // parent was removed by first pass and doesn't appear anymore + // as a child of anything (aka, they have become orphaned). + // Note, that thanks to the topological sorting, we can be sure + // one unique pass is enough. Without it, we may need to perform + // N (while loop) second passes. + foreach ($sorted as $key => $value) { + if (!str_contains($key, '==>')) { + $parent = 'NULL'; + $child = $key; + } else { + [$parent, $child] = explode('==>', $key); // TODO: Consider caching this in a property. + } + + if (isset($removed[$parent]) && !isset($remaining[$parent])) { + unset($sorted[$key]); + $removed[$child][$parent] = true; + unset($remaining[$child][$parent]); + // If this was the last parent of this child, remove it completely from the remaining array. + if (empty($remaining[$child])) { + unset($remaining[$child]); + } + } + } + + // We are done, let's return the reduced array. + return $sorted; + } + + + /** + * Sort the xhprof run pseudo-topologically, so all parents are always before their children. + * + * Note that this is not a proper, complex, recursive topological sorting algorithm, returning + * nodes that later have to be converted back to xhprof "pairs" but, instead, does the specific + * work to get those parent==>child (2 levels only) "pairs" sorted (parents always before children). + * + * @param array $info The xhprof run data, original array. + * + * @return array The xhprof run data, sorted array. + */ + protected function xhprof_topo_sort(array $info): array { + $sorted = []; + $visited = []; + $remaining = $info; + do { + $newremaining = []; + foreach ($remaining as $key => $value) { + // If we already have visited this element, we can skip it. + if (isset($visited[$key])) { + continue; + } + if (!str_contains($key, '==>')) { + // It's a root element, we can add it to the sorted array. + $sorted[$key] = $info[$key]; + $visited[$key] = true; + } else { + [$parent, $child] = explode('==>', $key); // TODO: Consider caching this in a property. + if (isset($visited[$parent])) { + // Parent already visited, we can add any children to the sorted array. + $sorted[$key] = $info[$key]; + $visited[$child] = true; + } else { + // Cannot add this yet, we need to wait for the parent. + $newremaining[$key] = $value; + } + } + } + // Protection against infinite loops. + if (count($remaining) === count($newremaining)) { + $remaining = []; // So we exit the do...while loop. + } else { + $remaining = $newremaining; // There is still work to do. + } + } while (count($remaining) > 0); + + // We are done, let's return the sorted array. + return $sorted; + } } /**