API: Remove explicit profiling
authorBrad Jorsch <bjorsch@wikimedia.org>
Tue, 10 Mar 2015 22:26:31 +0000 (18:26 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Tue, 10 Mar 2015 22:35:22 +0000 (18:35 -0400)
The profileIn/profileOut pair should be covered by the Xhprof profiling
of the method calls it was wrapping.

The profileDBIn/profileDBOut pair are covered by profiling done by the
Database class.

Nothing in extensions in Gerrit is calling anything besides the
profileIn/profileOut pair (and likely those are only to avoid core
formerly throwing exceptions from internal profileDBIn/profileDBOut
calls), and nothing in core or extensions-in-Gerrit is using the methods
for fetching profiling data.

The methods are left as stubs for now to allow for backwards
compatibility in extensions.

Change-Id: I05ba4e2762dc86d5e2bafc183dce701239b43f5c

RELEASE-NOTES-1.25
includes/api/ApiBase.php
includes/api/ApiMain.php
includes/api/ApiPageSet.php
includes/api/ApiQuery.php
includes/api/ApiQueryBase.php

index 9a2e33e..d1745d2 100644 (file)
@@ -272,6 +272,7 @@ production.
 * Added ApiBase::lacksSameOriginSecurity() to allow modules to easily check if
   the current request was sent with the 'callback' parameter (or any future
   method that breaks the same-origin policy).
+* Profiling methods in ApiBase are deprecated and no longer need to be called.
 * The following methods have been deprecated and may be removed in a future
   release:
   * ApiBase::getDescription
@@ -280,6 +281,14 @@ production.
   * ApiBase::makeHelpMsg
   * ApiBase::makeHelpArrayToString
   * ApiBase::makeHelpMsgParameters
+  * ApiBase::getModuleProfileName
+  * ApiBase::profileIn
+  * ApiBase::profileOut
+  * ApiBase::safeProfileOut
+  * ApiBase::getProfileTime
+  * ApiBase::profileDBIn
+  * ApiBase::profileDBOut
+  * ApiBase::getProfileDBTime
   * ApiFormatBase::setUnescapeAmps
   * ApiFormatBase::getWantsHelp
   * ApiFormatBase::setHelp
index b03782f..74e51c8 100644 (file)
@@ -32,9 +32,6 @@
  * Module parameters: Derived classes can define getAllowedParams() to specify
  *    which parameters to expect, how to parse and validate them.
  *
- * Profiling: various methods to allow keeping tabs on various tasks and their
- *    time costs
- *
  * Self-documentation: code to allow the API to document its own state
  *
  * @ingroup API
@@ -483,9 +480,7 @@ abstract class ApiBase extends ContextSource {
         */
        protected function getDB() {
                if ( !isset( $this->mSlaveDB ) ) {
-                       $this->profileDBIn();
                        $this->mSlaveDB = wfGetDB( DB_SLAVE, 'api' );
-                       $this->profileDBOut();
                }
 
                return $this->mSlaveDB;
@@ -1297,7 +1292,6 @@ abstract class ApiBase extends ContextSource {
         * @throws UsageException
         */
        public function dieUsage( $description, $errorCode, $httpRespCode = 0, $extradata = null ) {
-               Profiler::instance()->close();
                throw new UsageException(
                        $description,
                        $this->encodeParamName( $errorCode ),
@@ -1954,6 +1948,21 @@ abstract class ApiBase extends ContextSource {
                throw new MWException( "Internal error in $method: $message" );
        }
 
+       /**
+        * Write logging information for API features to a debug log, for usage
+        * analysis.
+        * @param string $feature Feature being used.
+        */
+       protected function logFeatureUsage( $feature ) {
+               $request = $this->getRequest();
+               $s = '"' . addslashes( $feature ) . '"' .
+                       ' "' . wfUrlencode( str_replace( ' ', '_', $this->getUser()->getName() ) ) . '"' .
+                       ' "' . $request->getIP() . '"' .
+                       ' "' . addslashes( $request->getHeader( 'Referer' ) ) . '"' .
+                       ' "' . addslashes( $this->getMain()->getUserAgent() ) . '"';
+               wfDebugLog( 'api-feature-usage', $s, 'private' );
+       }
+
        /**@}*/
 
        /************************************************************************//**
@@ -2189,162 +2198,6 @@ abstract class ApiBase extends ContextSource {
 
        /**@}*/
 
-       /************************************************************************//**
-        * @name   Profiling
-        * @{
-        */
-
-       /**
-        * Profiling: total module execution time
-        */
-       private $mTimeIn = 0, $mModuleTime = 0;
-       /** @var ScopedCallback */
-       private $profile;
-       /** @var ScopedCallback */
-       private $dbProfile;
-
-       /**
-        * Get the name of the module as shown in the profiler log
-        *
-        * @param DatabaseBase|bool $db
-        *
-        * @return string
-        */
-       public function getModuleProfileName( $db = false ) {
-               if ( $db ) {
-                       return 'API:' . $this->mModuleName . '-DB';
-               }
-
-               return 'API:' . $this->mModuleName;
-       }
-
-       /**
-        * Start module profiling
-        */
-       public function profileIn() {
-               if ( $this->mTimeIn !== 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Called twice without calling profileOut()' );
-               }
-               $this->mTimeIn = microtime( true );
-               $this->profile = Profiler::instance()->scopedProfileIn( $this->getModuleProfileName() );
-       }
-
-       /**
-        * End module profiling
-        */
-       public function profileOut() {
-               if ( $this->mTimeIn === 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Called without calling profileIn() first' );
-               }
-               if ( $this->mDBTimeIn !== 0 ) {
-                       ApiBase::dieDebug(
-                               __METHOD__,
-                               'Must be called after database profiling is done with profileDBOut()'
-                       );
-               }
-
-               $this->mModuleTime += microtime( true ) - $this->mTimeIn;
-               $this->mTimeIn = 0;
-               Profiler::instance()->scopedProfileOut( $this->profile );
-       }
-
-       /**
-        * When modules crash, sometimes it is needed to do a profileOut() regardless
-        * of the profiling state the module was in. This method does such cleanup.
-        */
-       public function safeProfileOut() {
-               if ( $this->mTimeIn !== 0 ) {
-                       if ( $this->mDBTimeIn !== 0 ) {
-                               $this->profileDBOut();
-                       }
-                       $this->profileOut();
-               }
-       }
-
-       /**
-        * Total time the module was executed
-        * @return float
-        */
-       public function getProfileTime() {
-               if ( $this->mTimeIn !== 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Called without calling profileOut() first' );
-               }
-
-               return $this->mModuleTime;
-       }
-
-       /**
-        * Profiling: database execution time
-        */
-       private $mDBTimeIn = 0, $mDBTime = 0;
-
-       /**
-        * Start module profiling
-        */
-       public function profileDBIn() {
-               if ( $this->mTimeIn === 0 ) {
-                       ApiBase::dieDebug(
-                               __METHOD__,
-                               'Must be called while profiling the entire module with profileIn()'
-                       );
-               }
-               if ( $this->mDBTimeIn !== 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Called twice without calling profileDBOut()' );
-               }
-               $this->mDBTimeIn = microtime( true );
-
-               $this->dbProfile = Profiler::instance()->scopedProfileIn( $this->getModuleProfileName( true ) );
-       }
-
-       /**
-        * End database profiling
-        */
-       public function profileDBOut() {
-               if ( $this->mTimeIn === 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Must be called while profiling ' .
-                               'the entire module with profileIn()' );
-               }
-               if ( $this->mDBTimeIn === 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Called without calling profileDBIn() first' );
-               }
-
-               $time = microtime( true ) - $this->mDBTimeIn;
-               $this->mDBTimeIn = 0;
-
-               $this->mDBTime += $time;
-               $this->getMain()->mDBTime += $time;
-               Profiler::instance()->scopedProfileOut( $this->dbProfile );
-       }
-
-       /**
-        * Total time the module used the database
-        * @return float
-        */
-       public function getProfileDBTime() {
-               if ( $this->mDBTimeIn !== 0 ) {
-                       ApiBase::dieDebug( __METHOD__, 'Called without calling profileDBOut() first' );
-               }
-
-               return $this->mDBTime;
-       }
-
-       /**
-        * Write logging information for API features to a debug log, for usage
-        * analysis.
-        * @param string $feature Feature being used.
-        */
-       protected function logFeatureUsage( $feature ) {
-               $request = $this->getRequest();
-               $s = '"' . addslashes( $feature ) . '"' .
-                       ' "' . wfUrlencode( str_replace( ' ', '_', $this->getUser()->getName() ) ) . '"' .
-                       ' "' . $request->getIP() . '"' .
-                       ' "' . addslashes( $request->getHeader( 'Referer' ) ) . '"' .
-                       ' "' . addslashes( $this->getMain()->getUserAgent() ) . '"';
-               wfDebugLog( 'api-feature-usage', $s, 'private' );
-       }
-
-       /**@}*/
-
        /************************************************************************//**
         * @name   Deprecated
         * @{
@@ -2795,6 +2648,71 @@ abstract class ApiBase extends ContextSource {
                return false;
        }
 
+       /**
+        * @deprecated since 1.25, always returns empty string
+        * @param DatabaseBase|bool $db
+        * @return string
+        */
+       public function getModuleProfileName( $db = false ) {
+               wfDeprecated( __METHOD__, '1.25' );
+               return '';
+       }
+
+       /**
+        * @deprecated since 1.25
+        */
+       public function profileIn() {
+               // No wfDeprecated() yet because extensions call this and might need to
+               // keep doing so for BC.
+       }
+
+       /**
+        * @deprecated since 1.25
+        */
+       public function profileOut() {
+               // No wfDeprecated() yet because extensions call this and might need to
+               // keep doing so for BC.
+       }
+
+       /**
+        * @deprecated since 1.25
+        */
+       public function safeProfileOut() {
+               wfDeprecated( __METHOD__, '1.25' );
+       }
+
+       /**
+        * @deprecated since 1.25, always returns 0
+        * @return float
+        */
+       public function getProfileTime() {
+               wfDeprecated( __METHOD__, '1.25' );
+               return 0;
+       }
+
+       /**
+        * @deprecated since 1.25
+        */
+       public function profileDBIn() {
+               wfDeprecated( __METHOD__, '1.25' );
+       }
+
+       /**
+        * @deprecated since 1.25
+        */
+       public function profileDBOut() {
+               wfDeprecated( __METHOD__, '1.25' );
+       }
+
+       /**
+        * @deprecated since 1.25, always returns 0
+        * @return float
+        */
+       public function getProfileDBTime() {
+               wfDeprecated( __METHOD__, '1.25' );
+               return 0;
+       }
+
        /**@}*/
 }
 
index 9dc2411..1feb485 100644 (file)
@@ -361,14 +361,11 @@ class ApiMain extends ApiBase {
         * Execute api request. Any errors will be handled if the API was called by the remote client.
         */
        public function execute() {
-               $this->profileIn();
                if ( $this->mInternalMode ) {
                        $this->executeAction();
                } else {
                        $this->executeActionWithErrorHandling();
                }
-
-               $this->profileOut();
        }
 
        /**
@@ -450,8 +447,6 @@ class ApiMain extends ApiBase {
                // Reset and print just the error message
                ob_clean();
 
-               // If the error occurred during printing, do a printer->profileOut()
-               $this->mPrinter->safeProfileOut();
                $this->printResult( true );
        }
 
@@ -771,7 +766,6 @@ class ApiMain extends ApiBase {
                // Printer may not be able to handle errors. This is particularly
                // likely if the module returns something for getCustomPrinter().
                if ( !$this->mPrinter->canPrintErrors() ) {
-                       $this->mPrinter->safeProfileOut();
                        $this->mPrinter = $this->createPrinterByName( self::API_DEFAULT_FORMAT );
                }
 
@@ -1040,10 +1034,8 @@ class ApiMain extends ApiBase {
                $this->checkAsserts( $params );
 
                // Execute
-               $module->profileIn();
                $module->execute();
                Hooks::run( 'APIAfterExecute', array( &$module ) );
-               $module->profileOut();
 
                $this->reportUnusedParams();
 
@@ -1194,13 +1186,10 @@ class ApiMain extends ApiBase {
 
                $this->getResult()->cleanUpUTF8();
                $printer = $this->mPrinter;
-               $printer->profileIn();
 
                $printer->initPrinter( false );
-
                $printer->execute();
                $printer->closePrinter();
-               $printer->profileOut();
        }
 
        /**
index e53e2b2..d462862 100644 (file)
@@ -115,11 +115,9 @@ class ApiPageSet extends ApiBase {
                $this->mAllowGenerator = ( $flags & ApiPageSet::DISABLE_GENERATORS ) == 0;
                $this->mDefaultNamespace = $defaultNamespace;
 
-               $this->profileIn();
                $this->mParams = $this->extractRequestParams();
                $this->mResolveRedirects = $this->mParams['redirects'];
                $this->mConvertTitles = $this->mParams['converttitles'];
-               $this->profileOut();
        }
 
        /**
@@ -143,17 +141,12 @@ class ApiPageSet extends ApiBase {
         *    relevant parameters as used
         */
        private function executeInternal( $isDryRun ) {
-               $this->profileIn();
-
                $generatorName = $this->mAllowGenerator ? $this->mParams['generator'] : null;
                if ( isset( $generatorName ) ) {
                        $dbSource = $this->mDbSource;
-                       $isQuery = $dbSource instanceof ApiQuery;
-                       if ( !$isQuery ) {
+                       if ( !$dbSource instanceof ApiQuery ) {
                                // If the parent container of this pageset is not ApiQuery, we must create it to run generator
                                $dbSource = $this->getMain()->getModuleManager()->getModule( 'query' );
-                               // Enable profiling for query module because it will be used for db sql profiling
-                               $dbSource->profileIn();
                        }
                        $generator = $dbSource->getModuleManager()->getModule( $generatorName, null, true );
                        if ( $generator === null ) {
@@ -174,9 +167,6 @@ class ApiPageSet extends ApiBase {
                        $tmpPageSet->executeInternal( $isDryRun );
 
                        // populate this pageset with the generator output
-                       $this->profileOut();
-                       $generator->profileIn();
-
                        if ( !$isDryRun ) {
                                $generator->executeGenerator( $this );
                                Hooks::run( 'APIQueryGeneratorAfterExecute', array( &$generator, &$this ) );
@@ -187,17 +177,10 @@ class ApiPageSet extends ApiBase {
                                        $main->getVal( $generator->encodeParamName( $paramName ) );
                                }
                        }
-                       $generator->profileOut();
-                       $this->profileIn();
 
                        if ( !$isDryRun ) {
                                $this->resolvePendingRedirects();
                        }
-
-                       if ( !$isQuery ) {
-                               // If this pageset is not part of the query, we called profileIn() above
-                               $dbSource->profileOut();
-                       }
                } else {
                        // Only one of the titles/pageids/revids is allowed at the same time
                        $dataSource = null;
@@ -241,7 +224,6 @@ class ApiPageSet extends ApiBase {
                                }
                        }
                }
-               $this->profileOut();
        }
 
        /**
@@ -678,9 +660,7 @@ class ApiPageSet extends ApiBase {
         * @param array $titles Array of Title objects
         */
        public function populateFromTitles( $titles ) {
-               $this->profileIn();
                $this->initFromTitles( $titles );
-               $this->profileOut();
        }
 
        /**
@@ -688,9 +668,7 @@ class ApiPageSet extends ApiBase {
         * @param array $pageIDs Array of page IDs
         */
        public function populateFromPageIDs( $pageIDs ) {
-               $this->profileIn();
                $this->initFromPageIds( $pageIDs );
-               $this->profileOut();
        }
 
        /**
@@ -703,9 +681,7 @@ class ApiPageSet extends ApiBase {
         * @param ResultWrapper $queryResult Query result object
         */
        public function populateFromQueryResult( $db, $queryResult ) {
-               $this->profileIn();
                $this->initFromQueryResult( $queryResult );
-               $this->profileOut();
        }
 
        /**
@@ -713,9 +689,7 @@ class ApiPageSet extends ApiBase {
         * @param array $revIDs Array of revision IDs
         */
        public function populateFromRevisionIDs( $revIDs ) {
-               $this->profileIn();
                $this->initFromRevIDs( $revIDs );
-               $this->profileOut();
        }
 
        /**
@@ -778,10 +752,8 @@ class ApiPageSet extends ApiBase {
                $set = $linkBatch->constructSet( 'page', $db );
 
                // Get pageIDs data from the `page` table
-               $this->profileDBIn();
                $res = $db->select( 'page', $this->getPageTableFields(), $set,
                        __METHOD__ );
-               $this->profileDBOut();
 
                // Hack: get the ns:titles stored in array(ns => array(titles)) format
                $this->initFromQueryResult( $res, $linkBatch->data, true ); // process Titles
@@ -812,10 +784,8 @@ class ApiPageSet extends ApiBase {
                        $db = $this->getDB();
 
                        // Get pageIDs data from the `page` table
-                       $this->profileDBIn();
                        $res = $db->select( 'page', $this->getPageTableFields(), $set,
                                __METHOD__ );
-                       $this->profileDBOut();
                }
 
                $this->initFromQueryResult( $res, $remaining, false ); // process PageIDs
@@ -921,7 +891,6 @@ class ApiPageSet extends ApiBase {
                        $where = array( 'rev_id' => $revids, 'rev_page = page_id' );
 
                        // Get pageIDs data from the `page` table
-                       $this->profileDBIn();
                        $res = $db->select( $tables, $fields, $where, __METHOD__ );
                        foreach ( $res as $row ) {
                                $revid = intval( $row->rev_id );
@@ -931,7 +900,6 @@ class ApiPageSet extends ApiBase {
                                $pageids[$pageid] = '';
                                unset( $remaining[$revid] );
                        }
-                       $this->profileDBOut();
                }
 
                $this->mMissingRevIDs = array_keys( $remaining );
@@ -948,7 +916,6 @@ class ApiPageSet extends ApiBase {
                        $fields = array( 'ar_rev_id', 'ar_namespace', 'ar_title' );
                        $where = array( 'ar_rev_id' => $this->mMissingRevIDs );
 
-                       $this->profileDBIn();
                        $res = $db->select( $tables, $fields, $where, __METHOD__ );
                        $titles = array();
                        foreach ( $res as $row ) {
@@ -956,7 +923,6 @@ class ApiPageSet extends ApiBase {
                                $titles[$revid] = Title::makeTitle( $row->ar_namespace, $row->ar_title );
                                unset( $remaining[$revid] );
                        }
-                       $this->profileDBOut();
 
                        $this->initFromTitles( $titles );
 
@@ -1012,9 +978,7 @@ class ApiPageSet extends ApiBase {
                                }
 
                                // Get pageIDs data from the `page` table
-                               $this->profileDBIn();
                                $res = $db->select( 'page', $pageFlds, $set, __METHOD__ );
-                               $this->profileDBOut();
 
                                // Hack: get the ns:titles stored in array(ns => array(titles)) format
                                $this->initFromQueryResult( $res, $linkBatch->data, true );
@@ -1033,7 +997,6 @@ class ApiPageSet extends ApiBase {
                $lb = new LinkBatch();
                $db = $this->getDB();
 
-               $this->profileDBIn();
                $res = $db->select(
                        'redirect',
                        array(
@@ -1045,7 +1008,6 @@ class ApiPageSet extends ApiBase {
                        ), array( 'rd_from' => array_keys( $this->mPendingRedirectIDs ) ),
                        __METHOD__
                );
-               $this->profileDBOut();
                foreach ( $res as $row ) {
                        $rdfrom = intval( $row->rd_from );
                        $from = $this->mPendingRedirectIDs[$rdfrom]->getPrefixedText();
index 9196dc7..f4b64a3 100644 (file)
@@ -169,9 +169,7 @@ class ApiQuery extends ApiBase {
         */
        public function getNamedDB( $name, $db, $groups ) {
                if ( !array_key_exists( $name, $this->mNamedDB ) ) {
-                       $this->profileDBIn();
                        $this->mNamedDB[$name] = wfGetDB( $db, $groups );
-                       $this->profileDBOut();
                }
 
                return $this->mNamedDB[$name];
@@ -295,10 +293,8 @@ class ApiQuery extends ApiBase {
                        $params = $module->extractRequestParams();
                        $cacheMode = $this->mergeCacheMode(
                                $cacheMode, $module->getCacheMode( $params ) );
-                       $module->profileIn();
                        $module->execute();
                        Hooks::run( 'APIQueryAfterExecute', array( &$module ) );
-                       $module->profileOut();
                }
 
                // Set the cache mode
index 7414913..1d4cff9 100644 (file)
@@ -372,12 +372,7 @@ abstract class ApiQueryBase extends ApiBase {
                        isset( $extraQuery['join_conds'] ) ? (array)$extraQuery['join_conds'] : array()
                );
 
-               // getDB has its own profileDBIn/Out calls
-               $db = $this->getDB();
-
-               $this->profileDBIn();
-               $res = $db->select( $tables, $fields, $where, $method, $options, $join_conds );
-               $this->profileDBOut();
+               $res = $this->getDB()->select( $tables, $fields, $where, $method, $options, $join_conds );
 
                return $res;
        }
@@ -590,7 +585,6 @@ abstract class ApiQueryBase extends ApiBase {
        protected function checkRowCount() {
                wfDeprecated( __METHOD__, '1.24' );
                $db = $this->getDB();
-               $this->profileDBIn();
                $rowcount = $db->estimateRowCount(
                        $this->tables,
                        $this->fields,
@@ -598,7 +592,6 @@ abstract class ApiQueryBase extends ApiBase {
                        __METHOD__,
                        $this->options
                );
-               $this->profileDBOut();
 
                if ( $rowcount > $this->getConfig()->get( 'APIMaxDBRows' ) ) {
                        return false;