Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Added simple time tracking panel #91

Closed
wants to merge 1 commit into from

Conversation

stekycz
Copy link

@stekycz stekycz commented Jan 5, 2015

Shows all timer results in time panel for easier time debugging. See http://forum.nette.org/cs/21752-vysledky-mereni-casu-v-debbuger-baru

Questions

  • Default listener for logging times of Application and Presenter should be in application I guess, right? Not related, see [WIP] Added simple time tracking panel #91 (comment)
  • Should disable option exist in NEON configuration (using CompilerExtension)?
  • What about time panel ID? Is it ok to name it as I did or should I use generated name in method addPanel as ID to get the panel?
  • Are all used information needed in table?
  • Is there some way (or should be) to create some "tree" of times? For example Application::onStartup could be collapsed hiding all times generated during this event (in all its listeners).

@JanTvrdik
Copy link
Contributor

I don't see a point in built-in measuring Application or Presenter. It will tell you nothing useful.

@stekycz
Copy link
Author

stekycz commented Jan 5, 2015

I think it will. There is a lot of things you can measure - action, handles, beforeRender, render, afterRender, shutdown etc. It could tell in which part of the application life cycle could be a bottle neck.

It is also taken from mentioned topic, see http://forum.nette.org/cs/21752-vysledky-mereni-casu-v-debbuger-baru#p148559

@JanTvrdik
Copy link
Contributor

@stekycz You should use a profiler for this. Measuring it with Tracy will slow down application and provide very little of actually useful data.

@stekycz
Copy link
Author

stekycz commented Jan 5, 2015

@JanTvrdik Yes, I use profiles when it is needed. I probably understand what you mean. This feature is probably for extra extension only for those who would like to use it.

@@ -379,7 +379,7 @@ public static function getBar()
{
if (!self::$bar) {
self::$bar = new Bar;
self::$bar->addPanel(new DefaultBarPanel('time'));
self::$bar->addPanel(new DefaultBarPanel('time'), 'time');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably use __CLASS__ . ':time' here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it could be better. I could also use default logic without need of own ID. However I am not sure what is better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom ID is better.

@JanTvrdik
Copy link
Contributor

Is there some way (or should be) to create some "tree" of times?

That would be cool, however it is not possible because the calls may look like this:

Debugger::timer('a');
Debugger::timer('b');
Debugger::timer('b');
Debugger::timer('b');
Debugger::timer('c');
Debugger::timer('a');
Debugger::timer('c');

I see no way how to render tree from this.

@stekycz
Copy link
Author

stekycz commented Jan 6, 2015

There could be some optional parameter with meaning of depth. If not specified it takes current internal depth. For example:

Debugger::timer('a', -1);
Debugger::timer('b');
Debugger::timer('b');
Debugger::timer('b');
Debugger::timer('c');
Debugger::timer('a', +1);
Debugger::timer('c');

@fprochazka
Copy link
Contributor

I'd suggest finishing the current implementation first. Improvements can be done separately :)

@JanTvrdik
Copy link
Contributor

There could be some optional parameter with meaning of depth.

No, there really could not be some magical optional parameter which nobody would use.


The biggest problem with the current implementation (assuming all may suggestions are accepted) is that the last two columns have confusing names. One is "since last" and the other is "since last with same name" but named just "delta". But "since last" is also a delta. I have no idea for better column names however.

@dg dg force-pushed the master branch 11 times, most recently from 9151150 to 00083f3 Compare February 10, 2015 03:47
@dg
Copy link
Member

dg commented Feb 10, 2015

Panel time was removed, but this can be nice extension.

@stekycz
Copy link
Author

stekycz commented Feb 10, 2015

@dg Do you mean extra extension or extension included in Nette? Current implementation (without time panel) could be updated to use this idea.

@dg
Copy link
Member

dg commented Feb 10, 2015

It is developed and maintained by you, so your extension.

@stekycz
Copy link
Author

stekycz commented Feb 10, 2015

Understood

@stekycz stekycz closed this Feb 10, 2015
@stekycz stekycz deleted the feature/time-panel branch February 10, 2015 12:10
@dg
Copy link
Member

dg commented Feb 10, 2015

When it will be mature and will prove that it is useful and you will be willing to do it, we can merge it to Tracy.

@stekycz
Copy link
Author

stekycz commented Feb 10, 2015

We will see :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants