From 0f8984f5a7005d3c0aae32394710846792885095 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 9 Feb 2013 15:47:55 -0800 Subject: [PATCH] Move XHProf sampling code out of index.php Summary: - Separate the ideas of "requested" (explicit user request) vs "started" (user request or sampling). - Move this code out of index.php into the XHProf stuff (general effort to make index.php smaller). Test Plan: Verified that profiling still works, and profiling extends to ajax requests. Set sampling rate to 2, saw 50% samples. Looked at database, saw sampling data populating properly. Reviewers: vrana, nh Reviewed By: vrana CC: aran Differential Revision: https://secure.phabricator.com/D4891 --- .../plugin/DarkConsoleXHProfPlugin.php | 3 +- .../xhprof/DarkConsoleXHProfPluginAPI.php | 138 ++++++++++++------ src/view/page/PhabricatorStandardPageView.php | 2 +- webroot/index.php | 20 +-- 4 files changed, 94 insertions(+), 69 deletions(-) diff --git a/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php b/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php index 4574056deb..0cae8541d2 100644 --- a/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php +++ b/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php @@ -97,8 +97,7 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin { public function willShutdown() { - if (DarkConsoleXHProfPluginAPI::isProfilerRequested() && - (DarkConsoleXHProfPluginAPI::isProfilerRequested() !== 'all')) { + if (DarkConsoleXHProfPluginAPI::isProfilerStarted()) { $this->xhprofID = DarkConsoleXHProfPluginAPI::stopProfiler(); } } diff --git a/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php b/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php index e2f9cad21d..afad7f1b8e 100644 --- a/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php +++ b/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php @@ -27,20 +27,32 @@ final class DarkConsoleXHProfPluginAPI { return $header; } - static $profilerRequested = null; + return false; + } - if (!isset($profilerRequested)) { + public static function shouldStartProfiler() { + if (self::isProfilerRequested()) { + return true; + } + + static $sample_request = null; + + if ($sample_request === null) { if (PhabricatorEnv::getEnvConfig('debug.profile-rate')) { $rate = PhabricatorEnv::getEnvConfig('debug.profile-rate'); if (mt_rand(1, $rate) == 1) { - $profilerRequested = true; + $sample_request = true; } else { - $profilerRequested = false; + $sample_request = false; } } } - return $profilerRequested; + return $sample_request; + } + + public static function isProfilerStarted() { + return self::$profilerStarted; } public static function includeXHProfLib() { @@ -56,8 +68,40 @@ final class DarkConsoleXHProfPluginAPI { } + public static function saveProfilerSample( + AphrontRequest $request, + $access_log) { + + if (!self::isProfilerStarted()) { + return; + } + + $profile = DarkConsoleXHProfPluginAPI::stopProfiler(); + $profile_sample = id(new PhabricatorXHProfSample()) + ->setFilePHID($profile); + + if (self::isProfilerRequested()) { + $sample_rate = 0; + } else { + $sample_rate = PhabricatorEnv::getEnvConfig('debug.profile-rate'); + } + + $profile_sample->setSampleRate($sample_rate); + + if ($access_log) { + $profile_sample + ->setUsTotal($access_log->getData('T')) + ->setHostname($access_log->getData('h')) + ->setRequestPath($access_log->getData('U')) + ->setController($access_log->getData('C')) + ->setUserPHID($request->getUser()->getPHID()); + } + + $profile_sample->save(); + } + public static function hookProfiler() { - if (!self::isProfilerRequested()) { + if (!self::shouldStartProfiler()) { return; } @@ -79,49 +123,49 @@ final class DarkConsoleXHProfPluginAPI { } public static function stopProfiler() { - if (self::$profilerStarted) { - $data = xhprof_disable(); - $data = serialize($data); - $file_class = 'PhabricatorFile'; - - // Since these happen on GET we can't do guarded writes. These also - // sometimes happen after we've disposed of the write guard; in this - // case we need to disable the whole mechanism. - - $use_scope = AphrontWriteGuard::isGuardActive(); - if ($use_scope) { - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - } else { - AphrontWriteGuard::allowDangerousUnguardedWrites(true); - } - - $caught = null; - try { - $file = call_user_func( - array($file_class, 'newFromFileData'), - $data, - array( - 'mime-type' => 'application/xhprof', - 'name' => 'profile.xhprof', - )); - } catch (Exception $ex) { - $caught = $ex; - } - - if ($use_scope) { - unset($unguarded); - } else { - AphrontWriteGuard::allowDangerousUnguardedWrites(false); - } - - if ($caught) { - throw $caught; - } else { - return $file->getPHID(); - } - } else { + if (!self::isProfilerStarted()) { return null; } + + $data = xhprof_disable(); + $data = serialize($data); + $file_class = 'PhabricatorFile'; + + // Since these happen on GET we can't do guarded writes. These also + // sometimes happen after we've disposed of the write guard; in this + // case we need to disable the whole mechanism. + + $use_scope = AphrontWriteGuard::isGuardActive(); + if ($use_scope) { + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + } else { + AphrontWriteGuard::allowDangerousUnguardedWrites(true); + } + + $caught = null; + try { + $file = call_user_func( + array($file_class, 'newFromFileData'), + $data, + array( + 'mime-type' => 'application/xhprof', + 'name' => 'profile.xhprof', + )); + } catch (Exception $ex) { + $caught = $ex; + } + + if ($use_scope) { + unset($unguarded); + } else { + AphrontWriteGuard::allowDangerousUnguardedWrites(false); + } + + if ($caught) { + throw $caught; + } else { + return $file->getPHID(); + } } } diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index ea9cc314da..80c7a37f5d 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -163,7 +163,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { require_celerity_resource('aphront-dark-console-css'); $headers = array(); - if (DarkConsoleXHProfPluginAPI::isProfilerRequested()) { + if (DarkConsoleXHProfPluginAPI::isProfilerStarted()) { $headers[DarkConsoleXHProfPluginAPI::getProfilerHeader()] = 'page'; } diff --git a/webroot/index.php b/webroot/index.php index 38e776f241..c6af0494f7 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -132,25 +132,7 @@ try { $access_log->write(); } - if (DarkConsoleXHProfPluginAPI::isProfilerRequested()) { - $profile = DarkConsoleXHProfPluginAPI::stopProfiler(); - $profile_sample = id(new PhabricatorXHProfSample()) - ->setFilePHID($profile); - if (empty($_REQUEST['__profile__'])) { - $sample_rate = PhabricatorEnv::getEnvConfig('debug.profile-rate'); - } else { - $sample_rate = 0; - } - $profile_sample->setSampleRate($sample_rate); - if ($access_log) { - $profile_sample->setUsTotal($access_log->getData('T')) - ->setHostname($access_log->getData('h')) - ->setRequestPath($access_log->getData('U')) - ->setController($access_log->getData('C')) - ->setUserPHID($request->getUser()->getPHID()); - } - $profile_sample->save(); - } + DarkConsoleXHProfPluginAPI::saveProfilerSample($request, $access_log); } catch (Exception $ex) { PhabricatorStartup::didFatal("[Exception] ".$ex->getMessage());