-
Notifications
You must be signed in to change notification settings - Fork 36
Conversation
src/Xhgui/Profiles.php
Outdated
@@ -22,6 +22,20 @@ public function __construct(MongoDb $db) | |||
*/ | |||
public function insert($profile) | |||
{ | |||
$profile = $this->setProfilingId($profile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your tabulation is off -- this project uses spaces, you use tabs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit: 8e46f10
src/Xhgui/Profiles.php
Outdated
$profile['_id'] = Xhgui_Saver::getLastProfilingId(); | ||
} | ||
return $profile; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could just turn this into a static getCurrentProfileId
method that provides static variable (initialized to new MongoId
on first call)
and then just call that from insert
and you'll have the ability to call it from anywhere that requires it.
Does this make sense to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense: 8e46f10#diff-4fe2f3dd6454d47d399256f2e1cbd7d7R38
src/Xhgui/Saver.php
Outdated
@@ -23,7 +28,20 @@ public static function factory($config) | |||
$mongo = new MongoClient($config['db.host'], $config['db.options']); | |||
$mongo->{$config['db.db']}->results->findOne(); | |||
$profiles = new Xhgui_Profiles($mongo->{$config['db.db']}); | |||
// Get new profiling ID, if profiler is enabled | |||
if (call_user_func($config['profiler.enable'])) { | |||
self::$lastProfilingId = new MongoId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think profiler.enable function should be called for anything except profiler enabling, as we won't know what lies there.
The user should be responsible for starting the profiler or take into account that asking for current profiling id while the profiler isn't running doesn't return anything meaningful (a MongoId that won't be persisted)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. And as I moved the static method into the Profiling.php
class, then there are no changes in Saver.php
class. I reverted my changes in a separate commit:
c68ee22
I'm not really clear on how/why this feature is needed. In what scenarios do you need this? |
@markstory This is so you can combine other profiling tools or manual profiling results with XHGUI results by having the profiling ID available. If there was a way to get some meaningful ID from xhprof without disabling it, I'd probably use that. |
8e46f10
to
909ffbc
Compare
I'd use |
Initiate profiling id in
Xhgui_Saver
class constructor. That allows to use the same profiling ID throughout your request and profiler results can be easily mapped back to the areas, where this ID is used.