From 5978bbfc64e266750eb5eb202f6c74f701c4f7a7 Mon Sep 17 00:00:00 2001 From: Nick Harper Date: Fri, 24 Aug 2012 15:14:38 -0700 Subject: [PATCH] Do sampled profiling of requests Summary: People have occasionally complained about phabricator being slow. We have the access log to look at to see when slowness happens, but it doesn't tell us much about why it happened. Since it's usually a sporadic issue that's reported, it's hard to reproduce and then profile. This change will allow us to collect sampled profiles so we can look at them when slowness occurs. Test Plan: checking that sampling works correctly: - set rate to 0; do several page loads; check no new entries in table - set rate to 1; check that there's a new row in the table for each page load - set rate to 10; check that some requests write to table and some don't check new ui for samples: - load /xhprof/list/all/, see a list with a lot of samples - load /xhprof/list/sampled/, see only sampled runs - load /xhprof/list/manual/, see only non-sampled runs - load /xhprof/list/my-runs/, se only my manual runs Reviewers: vrana, epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3458 --- conf/default.conf.php | 9 +- resources/sql/patches/xhprof.sql | 14 +++ src/__phutil_library_map__.php | 8 ++ ...AphrontDefaultApplicationConfiguration.php | 1 + .../xhprof/DarkConsoleXHProfPluginAPI.php | 19 ++-- .../PhabricatorXHProfSampleListController.php | 81 +++++++++++++++ .../xhprof/storage/PhabricatorXHProfDAO.php | 24 +++++ .../storage/PhabricatorXHProfSample.php | 28 ++++++ .../view/PhabricatorXHProfSampleListView.php | 98 +++++++++++++++++++ .../patch/PhabricatorBuiltinPatchList.php | 3 + webroot/index.php | 41 ++++---- 11 files changed, 296 insertions(+), 30 deletions(-) create mode 100644 resources/sql/patches/xhprof.sql create mode 100644 src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php create mode 100644 src/applications/xhprof/storage/PhabricatorXHProfDAO.php create mode 100644 src/applications/xhprof/storage/PhabricatorXHProfSample.php create mode 100644 src/applications/xhprof/view/PhabricatorXHProfSampleListView.php diff --git a/conf/default.conf.php b/conf/default.conf.php index b1754e2ff5..7d22bb5454 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -1273,9 +1273,12 @@ return array( // since the redirect happens in Javascript. 'debug.stop-on-redirect' => false, - // Enable this to always profile every page. This is very slow! You should - // only enable it when debugging. - 'debug.profile-every-request' => false, + // Set the rate for how often to do sampled profiling. On average, one + // request for every number of requests specified here will be sampled. + // Set this value to 0 to completely disable profiling. In a production + // environment, this value should either be set to 0 (to disable) or to + // a large number (to sample only a few requests). + 'debug.profile-rate' => 0, // -- Previews ------------------------------------------------------------- // diff --git a/resources/sql/patches/xhprof.sql b/resources/sql/patches/xhprof.sql new file mode 100644 index 0000000000..05f2e160cf --- /dev/null +++ b/resources/sql/patches/xhprof.sql @@ -0,0 +1,14 @@ +CREATE DATABASE {$NAMESPACE}_xhprof; +CREATE TABLE {$NAMESPACE}_xhprof.xhprof_sample ( + `id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + `filePHID` VARCHAR(64) NOT NULL COLLATE utf8_bin, + `sampleRate` INT NOT NULL, + `usTotal` BIGINT UNSIGNED NOT NULL, + `hostname` VARCHAR(255) COLLATE utf8_bin, + `requestPath` VARCHAR(255) COLLATE utf8_bin, + `controller` VARCHAR(255) COLLATE utf8_bin, + `userPHID` VARCHAR(64) COLLATE utf8_bin, + `dateCreated` BIGINT UNSIGNED NOT NULL, + `dateModified` BIGINT UNSIGNED NOT NULL, + UNIQUE KEY (filePHID) +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4bf0ad4b7d..ce0e83299c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1116,10 +1116,14 @@ phutil_register_library_map(array( 'PhabricatorXHPASTViewStreamController' => 'applications/xhpastview/controller/PhabricatorXHPASTViewStreamController.php', 'PhabricatorXHPASTViewTreeController' => 'applications/xhpastview/controller/PhabricatorXHPASTViewTreeController.php', 'PhabricatorXHProfController' => 'applications/xhprof/controller/PhabricatorXHProfController.php', + 'PhabricatorXHProfDAO' => 'applications/xhprof/storage/PhabricatorXHProfDAO.php', 'PhabricatorXHProfProfileController' => 'applications/xhprof/controller/PhabricatorXHProfProfileController.php', 'PhabricatorXHProfProfileSymbolView' => 'applications/xhprof/view/PhabricatorXHProfProfileSymbolView.php', 'PhabricatorXHProfProfileTopLevelView' => 'applications/xhprof/view/PhabricatorXHProfProfileTopLevelView.php', 'PhabricatorXHProfProfileView' => 'applications/xhprof/view/PhabricatorXHProfProfileView.php', + 'PhabricatorXHProfSample' => 'applications/xhprof/storage/PhabricatorXHProfSample.php', + 'PhabricatorXHProfSampleListController' => 'applications/xhprof/controller/PhabricatorXHProfSampleListController.php', + 'PhabricatorXHProfSampleListView' => 'applications/xhprof/view/PhabricatorXHProfSampleListView.php', 'PhameAllBlogListController' => 'applications/phame/controller/blog/list/PhameAllBlogListController.php', 'PhameAllPostListController' => 'applications/phame/controller/post/list/PhameAllPostListController.php', 'PhameBlog' => 'applications/phame/storage/PhameBlog.php', @@ -2205,10 +2209,14 @@ phutil_register_library_map(array( 'PhabricatorXHPASTViewStreamController' => 'PhabricatorXHPASTViewPanelController', 'PhabricatorXHPASTViewTreeController' => 'PhabricatorXHPASTViewPanelController', 'PhabricatorXHProfController' => 'PhabricatorController', + 'PhabricatorXHProfDAO' => 'PhabricatorLiskDAO', 'PhabricatorXHProfProfileController' => 'PhabricatorXHProfController', 'PhabricatorXHProfProfileSymbolView' => 'PhabricatorXHProfProfileView', 'PhabricatorXHProfProfileTopLevelView' => 'PhabricatorXHProfProfileView', 'PhabricatorXHProfProfileView' => 'AphrontView', + 'PhabricatorXHProfSample' => 'PhabricatorXHProfDAO', + 'PhabricatorXHProfSampleListController' => 'PhabricatorXHProfController', + 'PhabricatorXHProfSampleListView' => 'AphrontView', 'PhameAllBlogListController' => 'PhameBlogListBaseController', 'PhameAllPostListController' => 'PhamePostListBaseController', 'PhameBlog' => 'PhameDAO', diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index 9db2809c51..ef935fdf89 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -126,6 +126,7 @@ class AphrontDefaultApplicationConfiguration ), '/xhprof/' => array( + 'list/(?P[^/]+)/' => 'PhabricatorXHProfSampleListController', 'profile/(?P[^/]+)/' => 'PhabricatorXHProfProfileController', ), diff --git a/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php b/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php index 8ab312c4df..fa56ead547 100644 --- a/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php +++ b/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php @@ -34,11 +34,20 @@ final class DarkConsoleXHProfPluginAPI { return $_REQUEST['__profile__']; } - if (PhabricatorEnv::getEnvConfig('debug.profile-every-request')) { - return PhabricatorEnv::getEnvConfig('debug.profile-every-request'); + static $profilerRequested = null; + + if (!isset($profilerRequested)) { + if (PhabricatorEnv::getEnvConfig('debug.profile-rate')) { + $rate = PhabricatorEnv::getEnvConfig('debug.profile-rate'); + if (mt_rand(1, $rate) == 1) { + $profilerRequested = true; + } else { + $profilerRequested = false; + } + } } - return false; + return $profilerRequested; } public static function includeXHProfLib() { @@ -73,9 +82,7 @@ final class DarkConsoleXHProfPluginAPI { public static function startProfiler() { self::includeXHProfLib(); - // Note: HPHP's implementation of XHProf currently requires an argument - // to xhprof_enable() -- see Facebook Task #531011. - xhprof_enable(0); + xhprof_enable(); } public static function stopProfiler() { diff --git a/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php b/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php new file mode 100644 index 0000000000..2d00565e9c --- /dev/null +++ b/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php @@ -0,0 +1,81 @@ +view = $data['view']; + } + + public function processRequest() { + $request = $this->getRequest(); + + $pager = new AphrontPagerView(); + $pager->setOffset($request->getInt('page')); + + switch ($this->view) { + case 'sampled': + $clause = '`sampleRate` > 0'; + $show_type = false; + break; + case 'my-runs': + $clause = qsprintf( + id(new PhabricatorXHProfSample())->establishConnection('r'), + '`sampleRate` = 0 AND `userPHID` = %s', + $request->getUser()->getPHID()); + $show_type = false; + break; + case 'manual': + $clause = '`sampleRate` = 0'; + $show_type = false; + break; + case 'all': + default: + $clause = '1 = 1'; + $show_type = true; + break; + } + + $samples = id(new PhabricatorXHProfSample())->loadAllWhere( + '%Q ORDER BY dateCreated DESC LIMIT %d, %d', + $clause, + $pager->getOffset(), + $pager->getPageSize() + 1); + + $samples = $pager->sliceResults($samples); + $pager->setURI($request->getRequestURI(), 'page'); + + $table = new PhabricatorXHProfSampleListView(); + $table->setUser($request->getUser()); + $table->setSamples($samples); + $table->setShowType($show_type); + + $panel = new AphrontPanelView(); + $panel->setHeader('XHProf Samples'); + $panel->appendChild($table); + $panel->appendChild($pager); + + return $this->buildStandardPageResponse( + $panel, + array('title' => 'XHProf Samples')); + + } +} diff --git a/src/applications/xhprof/storage/PhabricatorXHProfDAO.php b/src/applications/xhprof/storage/PhabricatorXHProfDAO.php new file mode 100644 index 0000000000..4e0c9cfe3a --- /dev/null +++ b/src/applications/xhprof/storage/PhabricatorXHProfDAO.php @@ -0,0 +1,24 @@ +samples = $samples; + return $this; + } + + public function setUser(PhabricatorUser $user) { + $this->user = $user; + return $this; + } + + public function setShowType($show_type) { + $this->showType = $show_type; + } + + public function render() { + $rows = array(); + + if (!$this->user) { + throw new Exception("Call setUser() before rendering!"); + } + + $user_phids = mpull($this->samples, 'getUserPHID'); + $users = id(new PhabricatorObjectHandleData($user_phids))->loadObjects(); + foreach ($this->samples as $sample) { + $sample_link = phutil_render_tag( + 'a', + array( + 'href' => '/xhprof/profile/'.$sample->getFilePHID().'/', + ), + $sample->getFilePHID()); + if ($this->showType) { + if ($sample->getSampleRate() == 0) { + $sample_link .= ' (manual run)'; + } else { + $sample_link .= ' (sampled)'; + } + } + $rows[] = array( + $sample_link, + phabricator_datetime($sample->getDateCreated(), $this->user), + number_format($sample->getUsTotal())." \xCE\xBCs", + $sample->getHostname(), + $sample->getRequestPath(), + $sample->getController(), + idx($users, $sample->getUserPHID()), + ); + } + + $table = new AphrontTableView($rows); + $table->setHeaders( + array( + 'Sample', + 'Date', + 'Wall Time', + 'Hostname', + 'Request Path', + 'Controller', + 'User', + )); + $table->setColumnClasses( + array( + '', + '', + 'right', + 'wide wrap', + '', + '', + )); + + return $table->render(); + } + +} diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index c27b94aae5..f46621ea94 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -979,6 +979,9 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'pastepolicy.sql' => array( 'type' => 'sql', 'name' => $this->getPatchPath('pastepolicy.sql'), + 'xhprof.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('xhprof.sql'), ), ); } diff --git a/webroot/index.php b/webroot/index.php index fbabeb7e7c..307697ec73 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -228,27 +228,6 @@ $headers = array_merge($headers, $response->getHeaders()); $sink->writeHeaders($headers); -// TODO: This shouldn't be possible in a production-configured environment. -if (DarkConsoleXHProfPluginAPI::isProfilerRequested() && - DarkConsoleXHProfPluginAPI::isProfilerRequested() === 'all') { - $profile = DarkConsoleXHProfPluginAPI::stopProfiler(); - $profile = - '
'. - ''. - '>>> View Profile <<<'. - ''. - '
'; - if (strpos($response_string, '') !== false) { - $response_string = str_replace( - '', - ''.$profile, - $response_string); - } else { - $sink->writeData($profile); - } -} - $sink->writeData($response_string); if ($access_log) { @@ -260,6 +239,26 @@ if ($access_log) { $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(); +} + /** * @group aphront */