Navigation Menu

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

Add HTTP debug header, #593 #624

Closed
wants to merge 1 commit into from

Conversation

Thegennok
Copy link
Collaborator

As requested in #593, there is now the memory comsuption and the time elapsed in a HTTP header if perf_debug is true

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @pascalchevrel, @TheoChevalier and @flodolo to be potential reviewers

@flodolo
Copy link
Collaborator

flodolo commented Feb 14, 2016

Since the code is basically the same of logScriptPerformances, I think it makes more sense to only keep one function and pass a parameter to ask to log info in error_log, header, or both

. 'MB)';
$render_time = 'Elapsed time (s) '
. round((microtime(true) - $_SERVER['REQUEST_TIME_FLOAT']), 4);
header('Transvision-perf: ' . $memory . ' ; ' . $render_time);
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for an extra space before the semicolon.
nit for the description: http header -> HTTP header

@Thegennok
Copy link
Collaborator Author

He log in error_log and HTTP header in the same time now.

@flodolo
Copy link
Collaborator

flodolo commented Feb 14, 2016

Unfortunately this generates an error in the log:

PHP Warning:  Cannot modify header information - headers already se
nt by (output started at /home/flodolo/github/transvision/app/views/templates/base.php:162) in /home/flodolo/github/transvision/app/classes/Transvision/
Utils.php on line 363, referer: xxx

It should be OK to move the call from dispatcher to the base template (but need to be verified thoroughly)

@pascalchevrel
Copy link
Member

This is too simplistic an approch :)
We need all views to output that perf http header and we need to send that header before the real content otherwise we get an error about headers already sent.

The right approach is to split the function in 3:
getScriptPerformances() that returns an array of raw data
logScriptPerformances() that does what it does now but gets the data from getScriptPerformances()
outputPerformancesHHeader() that outputs the header and also gets the data from getScriptPerformances()

Then in the dispatcher, we use output buffering and at the end of the dispatcher, we print the buffers like that:

print $perf_header . $content;

That's actually moving what we do in base.php to the dispatcher.
The json view should also be updated to not die and only print.

I have a working patch so maybe I could take the bug.

@pascalchevrel pascalchevrel self-assigned this Feb 19, 2016
@flodolo
Copy link
Collaborator

flodolo commented Feb 22, 2016

Replaced by #637

@flodolo flodolo closed this Feb 22, 2016
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