Merge branch 'MDL-79285' of https://github.com/stronk7/moodle

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);
+    }
 }
 

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)) {

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,

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;
+    }
 }
 
 /**