From: Brad Jorsch Date: Tue, 10 Mar 2015 22:26:31 +0000 (-0400) Subject: API: Remove explicit profiling X-Git-Tag: 1.31.0-rc.0~12141 X-Git-Url: http://git.cyclocoop.org//%27%40script%40/%27?a=commitdiff_plain;h=bfe07bed33f353a6490899adea0408be2805b942;p=lhc%2Fweb%2Fwiklou.git API: Remove explicit profiling 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 --- diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25 index 9a2e33e0cf..d1745d23ec 100644 --- a/RELEASE-NOTES-1.25 +++ b/RELEASE-NOTES-1.25 @@ -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 diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index b03782fa06..74e51c8838 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -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; + } + /**@}*/ } diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 9dc2411849..1feb485237 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -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(); } /** diff --git a/includes/api/ApiPageSet.php b/includes/api/ApiPageSet.php index e53e2b26fd..d462862518 100644 --- a/includes/api/ApiPageSet.php +++ b/includes/api/ApiPageSet.php @@ -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(); diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php index 9196dc7333..f4b64a3fa5 100644 --- a/includes/api/ApiQuery.php +++ b/includes/api/ApiQuery.php @@ -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 diff --git a/includes/api/ApiQueryBase.php b/includes/api/ApiQueryBase.php index 7414913ddc..1d4cff9109 100644 --- a/includes/api/ApiQueryBase.php +++ b/includes/api/ApiQueryBase.php @@ -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;