gajus / xhprof.io

GUI to analyze the profiling data collected using XHProf – A Hierarchical Profiler for PHP.
http://xhprof.io/
Other
429 stars 103 forks source link

Moved all inclusion code to prepend.php, because script may die or other... #39

Closed thebeline closed 3 years ago

thebeline commented 11 years ago

...wise exit before this gets included/executed.

staabm commented 11 years ago

this will move xhprof's shutdown function at the very front and therefore may introduce problems with other shutdown functions (because xhprofs's shutdown function finishes the request...)

thebeline commented 11 years ago

It actually won't.

This creates a shutdown function that creates a shutdown function to call the shutdown function. :-D

Basically, we define the shutdown function we want to call. We set a shutdown function (that gets called first as shutdown) that is actually an anonymous function that then sets another shutdown function that is again an anonymous function that then calls our actual shutdown function.

This allows us to set the function at the very beginning, but ensure that it is called absolutely last.

thebeline commented 11 years ago

The reason for calling the shutdown function this way is we ensure that our shutdown function will be set no matter what else happens in the script. If the php closes the connection and dies before the full document is parsed, this will still occur, be set, and be handled.

To Fix: We need a weight option, that is next on my agenda. We also need a way to have the logs be set on the local machine, but be pulled in to a remote machine on demand. This will allow people, like ourselves, to have the profiler running on many servers, but the GUI running on only one.

I'll make a ticket for these.

thebeline commented 11 years ago

@gajus - One less level of abstraction. Sorry about that. Tested.

thebeline commented 11 years ago

@gajus - All set.

staabm commented 11 years ago

maybe the append.php should stay empty in the repos for users which update from a former version (so they don't get a fatal error)

thebeline commented 11 years ago

Do you feel this works? I added it back so we don't get a fatal, and I added an error_log to notify people that they are doin it wrong. :-D

thebeline commented 11 years ago

Noted, how else do you suggest we notify them of where they are including the file? Proposed solutions help....

thebeline commented 11 years ago

Also, where is @gajus?

staabm commented 11 years ago

In case it was called from cli I would output $_SERVER['SCRIPT_FILENAME'] otherwise your solution will do,

Great job!

thebeline commented 11 years ago

That is a good point. In that case it is not added from a .htaccess file, or vhost block, but is part of the php.ini config. Actually, that is a good indicator. I will make note of that.

thebeline commented 11 years ago

As such?

thebeline commented 11 years ago

I don't believe I agreed to anything specifically. And if the append.php file is included during cli, the append directive is in the php.ini file, in which case the server and request uri will not be helpful.. The only reason I put that in there is to help the use find where they may have the auto_append_file directive, as it may be in a special .htaccess file. In the event of it being in php.ini, the file will not help, unless manually included.

Either way, the concept was to include as little code as possible in the append.php file. Also, to be completely honest, I am loseing motivation to even care to do it the 'right' way, as the owner of the repo seems wholly uninterested in the patches progression.

I would like to be a part of it's development, but if @gajus is not interested, I find neither am I....

thebeline commented 11 years ago

@staabm @gajus That's about as much as I care at this point... Unless I see some interest in the help.

staabm commented 11 years ago

I also hope @gajus makes his point and will find some time to merge the PRs. @thebeline thanks for your work.

joehoyle commented 11 years ago

FWIW I am running off this branch which fixed my issue of calling exit in my script not collecting data. Thanks!

staabm commented 11 years ago

@joehoyle good point, nice to hear!

thebeline commented 11 years ago

Thats what I am confused about... This handles (as far as I can tell) all exit conditions except interpret failures. die, exit, uncaught exceptions, etc. As a matter of fact, I intend to write in a handler to report the exit status, among other things... but not on this branch. @joehoyle, keep an eye peeled, if this doesn't get pulled soon, there will be a whole different fork... This is too simple to stick around like this this long.

joehoyle commented 11 years ago

@thebeline ok great thank!s