MDL-78552 core: tidy up MDL_PERF constants

* all constans usable in ABORT_AFTER_CONFIG should be always defined * MDL_PERFDB and $PERF->logwrites not used after legacy log removal * MDL_PERF_TEST should be documented in codebase * deprecated warnings in shutdowb manager
VincentSD · Jun 25, 2023 · 3c25ccd · 3c25ccd
1 parent 3cd8474
commit 3c25ccd
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 35 deletions.
diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php
@@ -1703,7 +1703,7 @@ public function acquire_lock($key) {
         $after = microtime(true);
         if ($lock) {
             $this->locks[$key] = $lock;
-            if ((defined('MDL_PERF') && MDL_PERF) || $this->perfdebug) {
+            if (MDL_PERF || $this->perfdebug) {
                 \core\lock\timing_wrapper_lock_factory::record_lock_data($after, $before,
                         $this->get_definition()->get_id(), $key, $lock, $this->get_identifier() . $key);
             }
@@ -1748,7 +1748,7 @@ public function release_lock($key) {
         }
         if ($released && array_key_exists($key, $this->locks)) {
             unset($this->locks[$key]);
-            if ((defined('MDL_PERF') && MDL_PERF) || $this->perfdebug) {
+            if (MDL_PERF || $this->perfdebug) {
                 \core\lock\timing_wrapper_lock_factory::record_lock_released_data($this->get_identifier() . $key);
             }
         }

diff --git a/config-dist.php b/config-dist.php
@@ -422,9 +422,6 @@
 //   Capture performance profiling data
 //   define('MDL_PERF'  , true);
 //
-//   Capture additional data from DB
-//   define('MDL_PERFDB'  , true);
-//
 //   Print to log (for passive profiling of production servers)
 //   define('MDL_PERFTOLOG'  , true);
 //

diff --git a/install.php b/install.php
@@ -49,6 +49,10 @@
 define('PHPUNIT_TEST', false);
 define('IGNORE_COMPONENT_CACHE', true);
 define('MDL_PERF_TEST', false);
+define('MDL_PERF', false);
+define('MDL_PERFTOFOOT', false);
+define('MDL_PERFTOLOG', false);
+define('MDL_PERFINC', false);
 
 // Servers should define a default timezone in php.ini, but if they don't then make sure something is defined.
 if (!function_exists('date_default_timezone_set') or !function_exists('date_default_timezone_get')) {

diff --git a/lib/classes/lock/lock_config.php b/lib/classes/lock/lock_config.php
@@ -93,7 +93,7 @@ public static function get_lock_factory(string $type): \core\lock\lock_factory {
         }
 
         // If tracking performance, insert a timing wrapper to keep track of lock delays.
-        if ((defined('MDL_PERF') && MDL_PERF) || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
+        if (MDL_PERF || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
             $wrapper = new timing_wrapper_lock_factory($type, $lockfactory);
             $lockfactory = $wrapper;
         }

diff --git a/lib/classes/shutdown_manager.php b/lib/classes/shutdown_manager.php
@@ -208,25 +208,25 @@ protected static function request_shutdown() {
         }
 
         // Deal with perf logging.
-        if ((defined('MDL_PERF') && MDL_PERF) || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
+        if (MDL_PERF || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
             if ($apachereleasemem) {
                 error_log('Mem usage over '.$apachereleasemem.': marking Apache child for reaping.');
             }
-            if (defined('MDL_PERFTOLOG') && MDL_PERFTOLOG) {
+            if (MDL_PERFTOLOG) {
                 $perf = get_performance_info();
                 error_log("PERF: " . $perf['txt']);
             }
-            if (defined('MDL_PERFINC') && MDL_PERFINC) {
+            if (MDL_PERFINC) {
                 $inc = get_included_files();
                 $ts  = 0;
                 foreach ($inc as $f) {
                     if (preg_match(':^/:', $f)) {
                         $fs = filesize($f);
                         $ts += $fs;
                         $hfs = display_size($fs);
-                        error_log(substr($f, strlen($CFG->dirroot)) . " size: $fs ($hfs)", null, null, 0);
+                        error_log(substr($f, strlen($CFG->dirroot)) . " size: $fs ($hfs)");
                     } else {
-                        error_log($f , null, null, 0);
+                        error_log($f);
                     }
                 }
                 if ($ts > 0 ) {

diff --git a/lib/classes/task/logging_trait.php b/lib/classes/task/logging_trait.php
@@ -79,7 +79,7 @@ protected function log($message, $depth = 1) {
     protected function log_start($message, $depth = 0) {
         $this->log($message, $depth);
 
-        if (defined('MDL_PERFTOLOG') && MDL_PERFTOLOG) {
+        if (MDL_PERFTOLOG) {
             $this->tracestats->$depth = [
                 'mem' => memory_get_usage(),
                 'time' => microtime(),

diff --git a/lib/classes/task/logmanager.php b/lib/classes/task/logmanager.php
@@ -301,7 +301,7 @@ public static function finalise_log(bool $failed = false) {
                 self::$logpath,
                 $failed,
                 $DB->perf_get_reads() - self::$taskloginfo->dbread,
-                $DB->perf_get_writes() - self::$taskloginfo->dbwrite - $PERF->logwrites,
+                $DB->perf_get_writes() - self::$taskloginfo->dbwrite,
                 self::$taskloginfo->timestart,
                 microtime(true)
             );

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
@@ -9642,13 +9642,7 @@ function get_performance_info() {
         }
     }
 
-    if (!empty($PERF->logwrites)) {
-        $info['logwrites'] = $PERF->logwrites;
-        $info['html'] .= '<li class="logwrites col-sm-4">Log DB writes '.$info['logwrites'].'</li> ';
-        $info['txt'] .= 'logwrites: '.$info['logwrites'].' ';
-    }
-
-    $info['dbqueries'] = $DB->perf_get_reads().'/'.($DB->perf_get_writes() - $PERF->logwrites);
+    $info['dbqueries'] = $DB->perf_get_reads().'/'.$DB->perf_get_writes();
     $info['html'] .= '<li class="dbqueries col-sm-4">DB reads/writes: '.$info['dbqueries'].'</li> ';
     $info['txt'] .= 'db reads/writes: '.$info['dbqueries'].' ';
 

diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php
@@ -1511,9 +1511,9 @@ public function footer() {
 
         // Provide some performance info if required
         $performanceinfo = '';
-        if ((defined('MDL_PERF') && MDL_PERF) || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
+        if (MDL_PERF || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
             $perf = get_performance_info();
-            if ((defined('MDL_PERFTOFOOT') && MDL_PERFTOFOOT) || debugging() || $CFG->perfdebug > 7) {
+            if (MDL_PERFTOFOOT || debugging() || (!empty($CFG->perfdebug) && $CFG->perfdebug > 7)) {
                 $performanceinfo = $perf['html'];
             }
         }

diff --git a/lib/setup.php b/lib/setup.php
@@ -281,21 +281,25 @@
     define('PHPUNIT_TEST', false);
 }
 
-// Performance tests needs to always display performance info, even in redirections.
+// Performance tests needs to always display performance info, even in redirections;
+// MDL_PERF_TEST is used in https://github.com/moodlehq/moodle-performance-comparison scripts.
 if (!defined('MDL_PERF_TEST')) {
     define('MDL_PERF_TEST', false);
-} else {
-    // We force the ones we need.
-    if (!defined('MDL_PERF')) {
-        define('MDL_PERF', true);
-    }
-    if (!defined('MDL_PERFDB')) {
-        define('MDL_PERFDB', true);
-    }
-    if (!defined('MDL_PERFTOFOOT')) {
-        define('MDL_PERFTOFOOT', true);
-    }
 }
+// Make sure all MDL_PERF* constants are always defined.
+if (!defined('MDL_PERF')) {
+    define('MDL_PERF', MDL_PERF_TEST);
+}
+if (!defined('MDL_PERFTOFOOT')) {
+    define('MDL_PERFTOFOOT', MDL_PERF_TEST);
+}
+if (!defined('MDL_PERFTOLOG')) {
+    define('MDL_PERFTOLOG', false);
+}
+if (!defined('MDL_PERFINC')) {
+    define('MDL_PERFINC', false);
+}
+// Note that PHPUnit and Behat tests should pass with both MDL_PERF true and false.
 
 // When set to true MUC (Moodle caching) will be disabled as much as possible.
 // A special cache factory will be used to handle this situation and will use special "disabled" equivalents objects.

diff --git a/lib/setuplib.php b/lib/setuplib.php
@@ -1195,7 +1195,6 @@ function init_performance_info() {
     global $PERF, $CFG, $USER;
 
     $PERF = new stdClass();
-    $PERF->logwrites = 0;
     if (function_exists('microtime')) {
         $PERF->starttime = microtime();
     }