Chad has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/170977

Change subject: Profiler code cleanup
......................................................................

Profiler code cleanup

- Put Profiler and ProfileSection in their own files and rely on
  Autoloader to use them (maintenance has been using the autoloader
  here for some time--we don't profile the autoloader manually)
- This reduces overhead in WebStart/doMaintenance by only loading
  three functions at profiler initialization and defers until the
  first profiling call happens
- Inline callback functions in ProfilerSimpleText rather than having
  public static functions.

Change-Id: Idf27677c068c50b847152c523a33e7f0c33fdeeb
---
M includes/AutoLoader.php
M includes/WebStart.php
A includes/profiler/ProfileSection.php
M includes/profiler/Profiler.php
A includes/profiler/ProfilerFunctions.php
M includes/profiler/ProfilerSimpleText.php
M maintenance/doMaintenance.php
7 files changed, 152 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/77/170977/1

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 36a0fcb..6035f78 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -874,7 +874,7 @@
        'ProfilerSimpleUDP' => 'includes/profiler/ProfilerSimpleUDP.php',
        'ProfilerStandard' => 'includes/profiler/ProfilerStandard.php',
        'ProfilerStub' => 'includes/profiler/ProfilerStub.php',
-       'ProfileSection' => 'includes/profiler/Profiler.php',
+       'ProfileSection' => 'includes/profiler/ProfileSection.php',
        'TransactionProfiler' => 'includes/profiler/Profiler.php',
 
        # includes/rcfeed
diff --git a/includes/WebStart.php b/includes/WebStart.php
index cb35ee5..dd27f3d 100644
--- a/includes/WebStart.php
+++ b/includes/WebStart.php
@@ -58,8 +58,8 @@
        $IP = realpath( '.' ) ?: dirname( __DIR__ );
 }
 
-# Load the profiler
-require_once "$IP/includes/profiler/Profiler.php";
+# Grab profiling functions
+require_once "$IP/includes/profiler/ProfilerFunctions.php";
 $wgRUstart = wfGetRusage() ?: array();
 
 # Start the autoloader, so that extensions can derive classes from core files
diff --git a/includes/profiler/ProfileSection.php 
b/includes/profiler/ProfileSection.php
new file mode 100644
index 0000000..9e0b61e
--- /dev/null
+++ b/includes/profiler/ProfileSection.php
@@ -0,0 +1,60 @@
+<?php
+/**
+ * Function scope profiling assistant
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Profiler
+ */
+
+/**
+ * Class for handling function-scope profiling
+ *
+ * @since 1.22
+ */
+class ProfileSection {
+       protected $name; // string; method name
+       protected $enabled = false; // boolean; whether profiling is enabled
+
+       /**
+        * Begin profiling of a function and return an object that ends 
profiling of
+        * the function when that object leaves scope. As long as the object is 
not
+        * specifically linked to other objects, it will fall out of scope at 
the same
+        * moment that the function to be profiled terminates.
+        *
+        * This is typically called like:
+        * <code>$section = new ProfileSection( __METHOD__ );</code>
+        *
+        * @param string $name Name of the function to profile
+        */
+       public function __construct( $name ) {
+               $this->name = $name;
+               if ( Profiler::$__instance === null ) { // use this directly to 
reduce overhead
+                       Profiler::instance();
+               }
+               if ( !( Profiler::$__instance instanceof ProfilerStub ) ) {
+                       $this->enabled = true;
+                       Profiler::$__instance->profileIn( $this->name );
+               }
+       }
+
+       function __destruct() {
+               if ( $this->enabled ) {
+                       Profiler::$__instance->profileOut( $this->name );
+               }
+       }
+}
diff --git a/includes/profiler/Profiler.php b/includes/profiler/Profiler.php
index aaf899f..cd19ac8 100644
--- a/includes/profiler/Profiler.php
+++ b/includes/profiler/Profiler.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Base class and functions for profiling.
+ * Base class for profiling.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,88 +21,6 @@
  * @ingroup Profiler
  * @defgroup Profiler Profiler
  */
-
-/**
- * Get system resource usage of current request context.
- * Invokes the getrusage(2) system call, requesting RUSAGE_SELF if on PHP5
- * or RUSAGE_THREAD if on HHVM. Returns false if getrusage is not available.
- *
- * @since 1.24
- * @return array|bool Resource usage data or false if no data available.
- */
-function wfGetRusage() {
-       if ( !function_exists( 'getrusage' ) ) {
-               return false;
-       } elseif ( defined ( 'HHVM_VERSION' ) ) {
-               return getrusage( 2 /* RUSAGE_THREAD */ );
-       } else {
-               return getrusage( 0 /* RUSAGE_SELF */ );
-       }
-}
-
-/**
- * Begin profiling of a function
- * @param string $functionname Name of the function we will profile
- */
-function wfProfileIn( $functionname ) {
-       if ( Profiler::$__instance === null ) { // use this directly to reduce 
overhead
-               Profiler::instance();
-       }
-       if ( !( Profiler::$__instance instanceof ProfilerStub ) ) {
-               Profiler::$__instance->profileIn( $functionname );
-       }
-}
-
-/**
- * Stop profiling of a function
- * @param string $functionname Name of the function we have profiled
- */
-function wfProfileOut( $functionname = 'missing' ) {
-       if ( Profiler::$__instance === null ) { // use this directly to reduce 
overhead
-               Profiler::instance();
-       }
-       if ( !( Profiler::$__instance instanceof ProfilerStub ) ) {
-               Profiler::$__instance->profileOut( $functionname );
-       }
-}
-
-/**
- * Class for handling function-scope profiling
- *
- * @since 1.22
- */
-class ProfileSection {
-       protected $name; // string; method name
-       protected $enabled = false; // boolean; whether profiling is enabled
-
-       /**
-        * Begin profiling of a function and return an object that ends 
profiling of
-        * the function when that object leaves scope. As long as the object is 
not
-        * specifically linked to other objects, it will fall out of scope at 
the same
-        * moment that the function to be profiled terminates.
-        *
-        * This is typically called like:
-        * <code>$section = new ProfileSection( __METHOD__ );</code>
-        *
-        * @param string $name Name of the function to profile
-        */
-       public function __construct( $name ) {
-               $this->name = $name;
-               if ( Profiler::$__instance === null ) { // use this directly to 
reduce overhead
-                       Profiler::instance();
-               }
-               if ( !( Profiler::$__instance instanceof ProfilerStub ) ) {
-                       $this->enabled = true;
-                       Profiler::$__instance->profileIn( $this->name );
-               }
-       }
-
-       function __destruct() {
-               if ( $this->enabled ) {
-                       Profiler::$__instance->profileOut( $this->name );
-               }
-       }
-}
 
 /**
  * Profiler base class that defines the interface and some trivial 
functionality
diff --git a/includes/profiler/ProfilerFunctions.php 
b/includes/profiler/ProfilerFunctions.php
new file mode 100644
index 0000000..bc6de69
--- /dev/null
+++ b/includes/profiler/ProfilerFunctions.php
@@ -0,0 +1,66 @@
+<?php
+/**
+ * Core profiling functions. Have to exist before basically anything.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Profiler
+ */
+
+/**
+ * Get system resource usage of current request context.
+ * Invokes the getrusage(2) system call, requesting RUSAGE_SELF if on PHP5
+ * or RUSAGE_THREAD if on HHVM. Returns false if getrusage is not available.
+ *
+ * @since 1.24
+ * @return array|bool Resource usage data or false if no data available.
+ */
+function wfGetRusage() {
+       if ( !function_exists( 'getrusage' ) ) {
+               return false;
+       } elseif ( defined ( 'HHVM_VERSION' ) ) {
+               return getrusage( 2 /* RUSAGE_THREAD */ );
+       } else {
+               return getrusage( 0 /* RUSAGE_SELF */ );
+       }
+}
+
+/**
+ * Begin profiling of a function
+ * @param string $functionname Name of the function we will profile
+ */
+function wfProfileIn( $functionname ) {
+       if ( Profiler::$__instance === null ) { // use this directly to reduce 
overhead
+               Profiler::instance();
+       }
+       if ( !( Profiler::$__instance instanceof ProfilerStub ) ) {
+               Profiler::$__instance->profileIn( $functionname );
+       }
+}
+
+/**
+ * Stop profiling of a function
+ * @param string $functionname Name of the function we have profiled
+ */
+function wfProfileOut( $functionname = 'missing' ) {
+       if ( Profiler::$__instance === null ) { // use this directly to reduce 
overhead
+               Profiler::instance();
+       }
+       if ( !( Profiler::$__instance instanceof ProfilerStub ) ) {
+               Profiler::$__instance->profileOut( $functionname );
+       }
+}
diff --git a/includes/profiler/ProfilerSimpleText.php 
b/includes/profiler/ProfilerSimpleText.php
index cc18890..b083c5f 100644
--- a/includes/profiler/ProfilerSimpleText.php
+++ b/includes/profiler/ProfilerSimpleText.php
@@ -33,7 +33,6 @@
  */
 class ProfilerSimpleText extends ProfilerStandard {
        public $visible = false; /* Show as <PRE> or <!-- ? */
-       static private $out;
 
        public function __construct( $profileConfig ) {
                if ( isset( $profileConfig['visible'] ) && 
$profileConfig['visible'] ) {
@@ -43,37 +42,40 @@
        }
 
        public function logData() {
+               $out = '';
                if ( $this->mTemplated ) {
                        $this->close();
                        $totalReal = isset( $this->mCollated['-total'] )
                                ? $this->mCollated['-total']['real']
                                : 0; // profiling mismatch error?
-                       uasort( $this->mCollated, array( 'self', 'sort' ) );
-                       array_walk( $this->mCollated, array( 'self', 'format' 
), $totalReal );
+
+                       uasort( $this->mCollated, function( $a, $b ) {
+                               // sort descending by time elapsed
+                               return $a['real'] < $b['real'];
+                       } );
+
+                       array_walk( $this->mCollated,
+                               function( $item, $key ) use ( &$out, $totalReal 
) {
+                                       $perc = $totalReal ? $item['real'] / 
$totalReal * 100 : 0;
+                                       $out .= sprintf( "%6.2f%% %3.6f %6d - 
%s\n",
+                                               $perc, $item['real'], 
$item['count'], $key );
+                               }
+                       );
+
                        $contentType = $this->getContentType();
                        if ( PHP_SAPI === 'cli' ) {
-                               print "<!--\n" . self::$out . "\n-->\n";
+                               print "<!--\n" . $out . "\n-->\n";
                        } elseif ( $contentType === 'text/html' ) {
                                if ( $this->visible ) {
-                                       print '<pre>' . self::$out . '</pre>';
+                                       print '<pre>' . $out . '</pre>';
                                } else {
-                                       print "<!--\n" . self::$out . "\n-->\n";
+                                       print "<!--\n" . $out . "\n-->\n";
                                }
                        } elseif ( $contentType === 'text/javascript' ) {
-                               print "\n/*\n" . self::$out . "*/\n";
+                               print "\n/*\n" . $out . "*/\n";
                        } elseif ( $contentType === 'text/css' ) {
-                               print "\n/*\n" . self::$out . "*/\n";
+                               print "\n/*\n" . $out . "*/\n";
                        }
                }
-       }
-
-       static function sort( $a, $b ) {
-               return $a['real'] < $b['real']; /* sort descending by time 
elapsed */
-       }
-
-       static function format( $item, $key, $totalReal ) {
-               $perc = $totalReal ? $item['real'] / $totalReal * 100 : 0;
-               self::$out .= sprintf( "%6.2f%% %3.6f %6d - %s\n",
-                       $perc, $item['real'], $item['count'], $key );
        }
 }
diff --git a/maintenance/doMaintenance.php b/maintenance/doMaintenance.php
index 46844c9..e4380a7 100644
--- a/maintenance/doMaintenance.php
+++ b/maintenance/doMaintenance.php
@@ -56,8 +56,8 @@
 
 # Start the autoloader, so that extensions can derive classes from core files
 require_once "$IP/includes/AutoLoader.php";
-# Stub the profiler
-require_once "$IP/includes/profiler/Profiler.php";
+# Grab profiling functions
+require_once "$IP/includes/profiler/ProfilerFunctions.php";
 
 # Start the profiler
 $wgProfiler = array();

-- 
To view, visit https://gerrit.wikimedia.org/r/170977
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idf27677c068c50b847152c523a33e7f0c33fdeeb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Chad <ch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to