Closed (fixed)
Project:
jStats
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
17 Jan 2012 at 05:21 UTC
Updated:
2 Feb 2012 at 09:20 UTC
Jump to comment: Most recent file
Comments
Comment #1
mikeytown2 commentedComment #2
DeFr commentedI've been playing a bit with the patch, and it does improve the front end loading speed, but not as much as I would have thought: locally, in a non-loaded environment but using DNS resolution, the callback time goes down from ~90ms to ~65ms, including about 25ms DNS lookup. That's still a 33% performance improvement as far as end users are concerned, and nothing in the wild is using the json information returned by the callback right now, so, that looks good.
(By the way, Content-Length: 13 is the right thing to include the carriage return :-))
The only thing I'm wondering about that makes me put this back to CNW is the use of the ignore_user_abort() function. According to the documentation, called this way, without argument, it's supposed to only return the current value, without changing it, and the code does nothing with the return value, so I'm a bit lost about the use case. Should it be ignore_user_abort(TRUE); ? Or is the documentation on php.net somehow wrong ?
Comment #3
mikeytown2 commentedGood eye with ignore_user_abort(). The code should be
ignore_user_abort(TRUE);Comment #4
DeFr commentedRe-reading the code right before commiting it, there's another thing I'm wondering about: is there a specific reason to call
header('Connection: close');twice, once before ob_start and once after it ? Isn't the second call enough ?Comment #5
mikeytown2 commentedSetting headers twice is a hack. See CDN module code for details. In this case because it is the very first thing to get executed we could probably get rid of it. I would also test the output buffer as well; that might not be needed.
Comment #6
DeFr commentedOkay, I've just pushed db7ef78, which is your patch with only one Connection: Close header (there's no other header to override) and with
ignore_user_abort(TRUE).The input buffering code does seems to have an impact, it looks like cURL doesn't close the connection without it. I'm not sure completely sure it's not just the three flush more than the output buffer, but I've left it as is, at least that way it's easier to keep in sync with your project :-)
So, the only thing left to say is Thanks for the patch, and for trying out jStats !
Comment #7
mikeytown2 commentedCan you put this in the 6.x code as well?
Comment #8
mikeytown2 commentedComment #9
DeFr commentedSure, after two nice git cherry-pick there's now 93c3062d in the 6.x-1.x branch.