archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Fix double-stops on profiler sampling

Browse Source 
Summary:
For some time, we've stopped the profiler twice when it was invoked by the sampling mechanism. The first time it actually stops, and we write a profile. The second time it hadn't been started, so it returns empty and we write an invalid profile.

Instead, keep track of whether it is running or not, and don't stop it a second time.

Ref T2870.

Test Plan: Set sample rate to 1-in-3, observed valid sample profiles generate.

Reviewers: btrahan, chad

CC: aran

Maniphest Tasks: T2870

Differential Revision: https://secure.phabricator.com/D5534
This commit is contained in:
epriestley
2013-04-02 06:26:26 -07:00
parent cde1416446
commit c13f9d157b
4 changed files with 131 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files

189
src/__celerity_resource_map__.php
View File

@@ -1189,15 +1189,14 @@ celerity_register_resource_map(array(
),
'javelin-behavior-aphront-drag-and-drop-textarea' =>
array(
'uri' => '/res/853e33b9/rsrc/js/application/core/behavior-drag-and-drop-textarea.js',
'uri' => '/res/326d6c1c/rsrc/js/application/core/behavior-drag-and-drop-textarea.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-dom',
2 => 'phabricator-drag-and-drop-file-upload',
3 => 'phabricator-paste-file-upload',
4 => 'phabricator-textareautils',
3 => 'phabricator-textareautils',
),
'disk' => '/rsrc/js/application/core/behavior-drag-and-drop-textarea.js',
),
@@ -2879,7 +2878,7 @@ celerity_register_resource_map(array(
),
'phabricator-drag-and-drop-file-upload' =>
array(
'uri' => '/res/a4c31f49/rsrc/js/application/core/DragAndDropFileUpload.js',
'uri' => '/res/df7582fd/rsrc/js/application/core/DragAndDropFileUpload.js',
'type' => 'js',
'requires' =>
array(
@@ -3106,20 +3105,6 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/css/application/objectselector/object-selector.css',
),
'phabricator-paste-file-upload' =>
array(
'uri' => '/res/b0b8afd8/rsrc/js/application/core/PasteFileUpload.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-install',
1 => 'javelin-util',
2 => 'javelin-request',
3 => 'javelin-dom',
4 => 'javelin-uri',
),
'disk' => '/rsrc/js/application/core/PasteFileUpload.js',
),
'phabricator-pinboard-view-css' =>
array(
'uri' => '/res/61ecd7cf/rsrc/css/layout/phabricator-pinboard-view.css',
@@ -3859,7 +3844,7 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/c4fbf0c2/core.pkg.css',
'type' => 'css',
),
'def4c982' =>
'd95b69e5' =>
array(
'name' => 'core.pkg.js',
'symbols' =>
@@ -3873,34 +3858,33 @@ celerity_register_resource_map(array(
6 => 'javelin-behavior-refresh-csrf',
7 => 'javelin-behavior-phabricator-watch-anchor',
8 => 'javelin-behavior-phabricator-autofocus',
9 => 'phabricator-paste-file-upload',
10 => 'phabricator-menu-item',
11 => 'phabricator-dropdown-menu',
12 => 'javelin-behavior-phabricator-oncopy',
13 => 'phabricator-tooltip',
14 => 'javelin-behavior-phabricator-tooltips',
15 => 'phabricator-prefab',
16 => 'javelin-behavior-device',
17 => 'javelin-behavior-toggle-class',
18 => 'javelin-behavior-lightbox-attachments',
19 => 'phabricator-busy',
20 => 'javelin-aphlict',
21 => 'phabricator-notification',
22 => 'javelin-behavior-aphlict-listen',
23 => 'javelin-behavior-phabricator-search-typeahead',
24 => 'javelin-behavior-konami',
25 => 'javelin-behavior-aphlict-dropdown',
26 => 'javelin-behavior-history-install',
27 => 'javelin-behavior-phabricator-gesture',
28 => 'javelin-behavior-phabricator-active-nav',
29 => 'javelin-behavior-phabricator-nav',
30 => 'javelin-behavior-phabricator-remarkup-assist',
31 => 'phabricator-textareautils',
32 => 'phabricator-file-upload',
33 => 'javelin-behavior-global-drag-and-drop',
34 => 'javelin-behavior-phabricator-reveal-content',
9 => 'phabricator-menu-item',
10 => 'phabricator-dropdown-menu',
11 => 'javelin-behavior-phabricator-oncopy',
12 => 'phabricator-tooltip',
13 => 'javelin-behavior-phabricator-tooltips',
14 => 'phabricator-prefab',
15 => 'javelin-behavior-device',
16 => 'javelin-behavior-toggle-class',
17 => 'javelin-behavior-lightbox-attachments',
18 => 'phabricator-busy',
19 => 'javelin-aphlict',
20 => 'phabricator-notification',
21 => 'javelin-behavior-aphlict-listen',
22 => 'javelin-behavior-phabricator-search-typeahead',
23 => 'javelin-behavior-konami',
24 => 'javelin-behavior-aphlict-dropdown',
25 => 'javelin-behavior-history-install',
26 => 'javelin-behavior-phabricator-gesture',
27 => 'javelin-behavior-phabricator-active-nav',
28 => 'javelin-behavior-phabricator-nav',
29 => 'javelin-behavior-phabricator-remarkup-assist',
30 => 'phabricator-textareautils',
31 => 'phabricator-file-upload',
32 => 'javelin-behavior-global-drag-and-drop',
33 => 'javelin-behavior-phabricator-reveal-content',
),
'uri' => '/res/pkg/def4c982/core.pkg.js',
'uri' => '/res/pkg/d95b69e5/core.pkg.js',
'type' => 'js',
),
'dca4a03d' =>
@@ -3936,7 +3920,7 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/8aaacd1b/differential.pkg.css',
'type' => 'css',
),
'68d5f4ba' =>
'e96b08f8' =>
array(
'name' => 'differential.pkg.js',
'symbols' =>
@@ -3962,7 +3946,7 @@ celerity_register_resource_map(array(
18 => 'javelin-behavior-differential-toggle-files',
19 => 'javelin-behavior-differential-user-select',
),
'uri' => '/res/pkg/68d5f4ba/differential.pkg.js',
'uri' => '/res/pkg/e96b08f8/differential.pkg.js',
'type' => 'js',
),
'c8ce2d88' =>
@@ -4059,7 +4043,7 @@ celerity_register_resource_map(array(
'aphront-typeahead-control-css' => 'c4fbf0c2',
'differential-changeset-view-css' => '8aaacd1b',
'differential-core-view-css' => '8aaacd1b',
'differential-inline-comment-editor' => '68d5f4ba',
'differential-inline-comment-editor' => 'e96b08f8',
'differential-local-commits-view-css' => '8aaacd1b',
'differential-results-table-css' => '8aaacd1b',
'differential-revision-add-comment-css' => '8aaacd1b',
@@ -4072,58 +4056,58 @@ celerity_register_resource_map(array(
'diffusion-icons-css' => 'c8ce2d88',
'global-drag-and-drop-css' => 'c4fbf0c2',
'inline-comment-summary-css' => '8aaacd1b',
'javelin-aphlict' => 'def4c982',
'javelin-aphlict' => 'd95b69e5',
'javelin-behavior' => 'fe22443b',
'javelin-behavior-aphlict-dropdown' => 'def4c982',
'javelin-behavior-aphlict-listen' => 'def4c982',
'javelin-behavior-aphront-basic-tokenizer' => 'def4c982',
'javelin-behavior-aphront-drag-and-drop' => '68d5f4ba',
'javelin-behavior-aphront-drag-and-drop-textarea' => '68d5f4ba',
'javelin-behavior-aphront-form-disable-on-submit' => 'def4c982',
'javelin-behavior-aphlict-dropdown' => 'd95b69e5',
'javelin-behavior-aphlict-listen' => 'd95b69e5',
'javelin-behavior-aphront-basic-tokenizer' => 'd95b69e5',
'javelin-behavior-aphront-drag-and-drop' => 'e96b08f8',
'javelin-behavior-aphront-drag-and-drop-textarea' => 'e96b08f8',
'javelin-behavior-aphront-form-disable-on-submit' => 'd95b69e5',
'javelin-behavior-audit-preview' => 'f96657b8',
'javelin-behavior-dark-console' => 'dca4a03d',
'javelin-behavior-device' => 'def4c982',
'javelin-behavior-differential-accept-with-errors' => '68d5f4ba',
'javelin-behavior-differential-add-reviewers-and-ccs' => '68d5f4ba',
'javelin-behavior-differential-comment-jump' => '68d5f4ba',
'javelin-behavior-differential-diff-radios' => '68d5f4ba',
'javelin-behavior-differential-dropdown-menus' => '68d5f4ba',
'javelin-behavior-differential-edit-inline-comments' => '68d5f4ba',
'javelin-behavior-differential-feedback-preview' => '68d5f4ba',
'javelin-behavior-differential-keyboard-navigation' => '68d5f4ba',
'javelin-behavior-differential-populate' => '68d5f4ba',
'javelin-behavior-differential-show-more' => '68d5f4ba',
'javelin-behavior-differential-toggle-files' => '68d5f4ba',
'javelin-behavior-differential-user-select' => '68d5f4ba',
'javelin-behavior-device' => 'd95b69e5',
'javelin-behavior-differential-accept-with-errors' => 'e96b08f8',
'javelin-behavior-differential-add-reviewers-and-ccs' => 'e96b08f8',
'javelin-behavior-differential-comment-jump' => 'e96b08f8',
'javelin-behavior-differential-diff-radios' => 'e96b08f8',
'javelin-behavior-differential-dropdown-menus' => 'e96b08f8',
'javelin-behavior-differential-edit-inline-comments' => 'e96b08f8',
'javelin-behavior-differential-feedback-preview' => 'e96b08f8',
'javelin-behavior-differential-keyboard-navigation' => 'e96b08f8',
'javelin-behavior-differential-populate' => 'e96b08f8',
'javelin-behavior-differential-show-more' => 'e96b08f8',
'javelin-behavior-differential-toggle-files' => 'e96b08f8',
'javelin-behavior-differential-user-select' => 'e96b08f8',
'javelin-behavior-diffusion-commit-graph' => 'f96657b8',
'javelin-behavior-diffusion-pull-lastmodified' => 'f96657b8',
'javelin-behavior-error-log' => 'dca4a03d',
'javelin-behavior-global-drag-and-drop' => 'def4c982',
'javelin-behavior-history-install' => 'def4c982',
'javelin-behavior-konami' => 'def4c982',
'javelin-behavior-lightbox-attachments' => 'def4c982',
'javelin-behavior-load-blame' => '68d5f4ba',
'javelin-behavior-global-drag-and-drop' => 'd95b69e5',
'javelin-behavior-history-install' => 'd95b69e5',
'javelin-behavior-konami' => 'd95b69e5',
'javelin-behavior-lightbox-attachments' => 'd95b69e5',
'javelin-behavior-load-blame' => 'e96b08f8',
'javelin-behavior-maniphest-batch-selector' => 'f85eb6d8',
'javelin-behavior-maniphest-subpriority-editor' => 'f85eb6d8',
'javelin-behavior-maniphest-transaction-controls' => 'f85eb6d8',
'javelin-behavior-maniphest-transaction-expand' => 'f85eb6d8',
'javelin-behavior-maniphest-transaction-preview' => 'f85eb6d8',
'javelin-behavior-phabricator-active-nav' => 'def4c982',
'javelin-behavior-phabricator-autofocus' => 'def4c982',
'javelin-behavior-phabricator-gesture' => 'def4c982',
'javelin-behavior-phabricator-keyboard-shortcuts' => 'def4c982',
'javelin-behavior-phabricator-nav' => 'def4c982',
'javelin-behavior-phabricator-object-selector' => '68d5f4ba',
'javelin-behavior-phabricator-oncopy' => 'def4c982',
'javelin-behavior-phabricator-remarkup-assist' => 'def4c982',
'javelin-behavior-phabricator-reveal-content' => 'def4c982',
'javelin-behavior-phabricator-search-typeahead' => 'def4c982',
'javelin-behavior-phabricator-tooltips' => 'def4c982',
'javelin-behavior-phabricator-watch-anchor' => 'def4c982',
'javelin-behavior-refresh-csrf' => 'def4c982',
'javelin-behavior-repository-crossreference' => '68d5f4ba',
'javelin-behavior-toggle-class' => 'def4c982',
'javelin-behavior-workflow' => 'def4c982',
'javelin-behavior-phabricator-active-nav' => 'd95b69e5',
'javelin-behavior-phabricator-autofocus' => 'd95b69e5',
'javelin-behavior-phabricator-gesture' => 'd95b69e5',
'javelin-behavior-phabricator-keyboard-shortcuts' => 'd95b69e5',
'javelin-behavior-phabricator-nav' => 'd95b69e5',
'javelin-behavior-phabricator-object-selector' => 'e96b08f8',
'javelin-behavior-phabricator-oncopy' => 'd95b69e5',
'javelin-behavior-phabricator-remarkup-assist' => 'd95b69e5',
'javelin-behavior-phabricator-reveal-content' => 'd95b69e5',
'javelin-behavior-phabricator-search-typeahead' => 'd95b69e5',
'javelin-behavior-phabricator-tooltips' => 'd95b69e5',
'javelin-behavior-phabricator-watch-anchor' => 'd95b69e5',
'javelin-behavior-refresh-csrf' => 'd95b69e5',
'javelin-behavior-repository-crossreference' => 'e96b08f8',
'javelin-behavior-toggle-class' => 'd95b69e5',
'javelin-behavior-workflow' => 'd95b69e5',
'javelin-dom' => 'fe22443b',
'javelin-event' => 'fe22443b',
'javelin-install' => 'fe22443b',
@@ -4145,39 +4129,38 @@ celerity_register_resource_map(array(
'lightbox-attachment-css' => 'c4fbf0c2',
'maniphest-task-summary-css' => '6b1fccc6',
'maniphest-transaction-detail-css' => '6b1fccc6',
'phabricator-busy' => 'def4c982',
'phabricator-busy' => 'd95b69e5',
'phabricator-content-source-view-css' => '8aaacd1b',
'phabricator-core-buttons-css' => 'c4fbf0c2',
'phabricator-core-css' => 'c4fbf0c2',
'phabricator-crumbs-view-css' => 'c4fbf0c2',
'phabricator-directory-css' => 'c4fbf0c2',
'phabricator-drag-and-drop-file-upload' => '68d5f4ba',
'phabricator-dropdown-menu' => 'def4c982',
'phabricator-file-upload' => 'def4c982',
'phabricator-drag-and-drop-file-upload' => 'e96b08f8',
'phabricator-dropdown-menu' => 'd95b69e5',
'phabricator-file-upload' => 'd95b69e5',
'phabricator-filetree-view-css' => 'c4fbf0c2',
'phabricator-flag-css' => 'c4fbf0c2',
'phabricator-form-view-css' => 'c4fbf0c2',
'phabricator-header-view-css' => 'c4fbf0c2',
'phabricator-jump-nav' => 'c4fbf0c2',
'phabricator-keyboard-shortcut' => 'def4c982',
'phabricator-keyboard-shortcut-manager' => 'def4c982',
'phabricator-keyboard-shortcut' => 'd95b69e5',
'phabricator-keyboard-shortcut-manager' => 'd95b69e5',
'phabricator-main-menu-view' => 'c4fbf0c2',
'phabricator-menu-item' => 'def4c982',
'phabricator-menu-item' => 'd95b69e5',
'phabricator-nav-view-css' => 'c4fbf0c2',
'phabricator-notification' => 'def4c982',
'phabricator-notification' => 'd95b69e5',
'phabricator-notification-css' => 'c4fbf0c2',
'phabricator-notification-menu-css' => 'c4fbf0c2',
'phabricator-object-item-list-view-css' => 'c4fbf0c2',
'phabricator-object-selector-css' => '8aaacd1b',
'phabricator-paste-file-upload' => 'def4c982',
'phabricator-prefab' => 'def4c982',
'phabricator-prefab' => 'd95b69e5',
'phabricator-project-tag-css' => '6b1fccc6',
'phabricator-remarkup-css' => 'c4fbf0c2',
'phabricator-shaped-request' => '68d5f4ba',
'phabricator-shaped-request' => 'e96b08f8',
'phabricator-side-menu-view-css' => 'c4fbf0c2',
'phabricator-standard-page-view' => 'c4fbf0c2',
'phabricator-textareautils' => 'def4c982',
'phabricator-tooltip' => 'def4c982',
'phabricator-textareautils' => 'd95b69e5',
'phabricator-tooltip' => 'd95b69e5',
'phabricator-transaction-view-css' => 'c4fbf0c2',
'phabricator-zindex-css' => 'c4fbf0c2',
'sprite-apps-large-css' => 'c4fbf0c2',

14
src/aphront/console/plugin/DarkConsoleXHProfPlugin.php
View File

@@ -5,7 +5,7 @@
*/
final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
protected $xhprofID;
protected $profileFilePHID;
public function getName() {
return 'XHProf';
@@ -13,7 +13,7 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
public function getColor() {
$data = $this->getData();
if ($data['xhprofID']) {
if ($data['profileFilePHID']) {
return '#ff00ff';
}
return null;
@@ -25,7 +25,7 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
public function generateData() {
return array(
'xhprofID' => $this->xhprofID,
'profileFilePHID' => $this->profileFilePHID,
'profileURI' => (string)$this
->getRequestURI()
->alter('__profile__', 'page'),
@@ -33,13 +33,13 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
}
public function getXHProfRunID() {
return $this->xhprofID;
return $this->profileFilePHID;
}
public function renderPanel() {
$data = $this->getData();
$run = $data['xhprofID'];
$run = $data['profileFilePHID'];
$profile_uri = $data['profileURI'];
if (!DarkConsoleXHProfPluginAPI::isProfilerAvailable()) {
@@ -101,9 +101,7 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
public function willShutdown() {
if (DarkConsoleXHProfPluginAPI::isProfilerStarted()) {
$this->xhprofID = DarkConsoleXHProfPluginAPI::stopProfiler();
}
$this->profileFilePHID = DarkConsoleXHProfPluginAPI::getProfileFilePHID();
}
}

57
src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php
View File

@@ -8,6 +8,8 @@
final class DarkConsoleXHProfPluginAPI {
private static $profilerStarted;
private static $profilerRunning;
private static $profileFilePHID;
public static function isProfilerAvailable() {
return extension_loaded('xhprof');
@@ -30,7 +32,7 @@ final class DarkConsoleXHProfPluginAPI {
return false;
}
public static function shouldStartProfiler() {
private static function shouldStartProfiler() {
if (self::isProfilerRequested()) {
return true;
}
@@ -55,6 +57,10 @@ final class DarkConsoleXHProfPluginAPI {
return self::$profilerStarted;
}
private static function isProfilerRunning() {
return self::$profilerRunning;
}
public static function includeXHProfLib() {
// TODO: this is incredibly stupid, but we may not have Phutil metamodule
// stuff loaded yet so we can't just phutil_get_library_root() our way
@@ -69,22 +75,19 @@ final class DarkConsoleXHProfPluginAPI {
public static function saveProfilerSample(PhutilDeferredLog $access_log) {
if (!self::isProfilerStarted()) {
$file_phid = self::getProfileFilePHID();
if (!$file_phid) {
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
$profile_sample = id(new PhabricatorXHProfSample())
->setFilePHID($file_phid)
->setSampleRate($sample_rate)
->setUsTotal($access_log->getData('T'))
->setHostname($access_log->getData('h'))
@@ -92,7 +95,18 @@ final class DarkConsoleXHProfPluginAPI {
->setController($access_log->getData('C'))
->setUserPHID($access_log->getData('P'));
$profile_sample->save();
AphrontWriteGuard::allowDangerousUnguardedWrites(true);
$caught = null;
try {
$profile_sample->save();
} catch (Exception $ex) {
$caught = $ex;
}
AphrontWriteGuard::allowDangerousUnguardedWrites(false);
if ($caught) {
throw $caught;
}
}
public static function hookProfiler() {
@@ -109,22 +123,29 @@ final class DarkConsoleXHProfPluginAPI {
}
self::startProfiler();
self::$profilerStarted = true;
}
public static function startProfiler() {
private static function startProfiler() {
self::includeXHProfLib();
xhprof_enable();
self::$profilerStarted = true;
self::$profilerRunning = true;
}
public static function stopProfiler() {
if (!self::isProfilerStarted()) {
return null;
public static function getProfileFilePHID() {
self::stopProfiler();
return self::$profileFilePHID;
}
private static function stopProfiler() {
if (!self::isProfilerRunning()) {
return;
}
$data = xhprof_disable();
$data = serialize($data);
$file_class = 'PhabricatorFile';
self::$profilerRunning = false;
// 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
@@ -140,7 +161,7 @@ final class DarkConsoleXHProfPluginAPI {
$caught = null;
try {
$file = call_user_func(
array($file_class, 'newFromFileData'),
array('PhabricatorFile', 'newFromFileData'),
$data,
array(
'mime-type' => 'application/xhprof',
@@ -158,9 +179,9 @@ final class DarkConsoleXHProfPluginAPI {
if ($caught) {
throw $caught;
} else {
return $file->getPHID();
}
self::$profileFilePHID = $file->getPHID();
}
}

1
webroot/index.php
View File

@@ -136,7 +136,6 @@ try {
));
DarkConsoleXHProfPluginAPI::saveProfilerSample($access_log);
} catch (Exception $ex) {
PhabricatorStartup::didFatal("[Exception] ".$ex->getMessage());
}