Adds a static for variable_get(), makes format_date() between 1/4 to 1/3 cheaper. Explanation of why here: http://drupal.org/node/513984#comment-1877252

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

jrchamp’s picture

Status: Needs work » Needs review
FileSize
3.67 KB

The variable_set in the setup function for the unit test wasn't setting the variables likely because format_date is popular enough to be called prior to its execution. Rerolled to get this moving.

Status: Needs review » Needs work

The last submitted patch failed testing.

jrchamp’s picture

Status: Needs work » Needs review
FileSize
4.49 KB

Same thing as #2, only with the clear applied to a different test.

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

Code looks, and bot likes it.

catch’s picture

Status: Reviewed & tested by the community » Needs work

OK so Moshe pointed out some issues measuring function calls with xdebug profiler (since it logs function calls specifically, but not language constructs etc.), and I found out xdebug adds overhead to function calls even if just the extension itself is enabled, which messes up my numbers a bit (not all of them on every issue, but I think it does in this case...).

Did a microbenchmark of 1 million calls to format_date() (attached the script, you'll need to change the format_date_new() function signature for the various alternatives).

Patch, xdebug disabled:

nothing: 0.244761943817 seconds
function no_op(): 0.624433040619 seconds
format_date() HEAD: 62.5531749725 seconds
format_date() patch: 63.6565389633 seconds

Patch: xdebug enabled:

nothing: 0.363921880722 seconds
function no_op(): 3.37409496307 seconds
format_date() HEAD: 107.372431993 seconds
format_date() patch: 99.9250171185 seconds

Patch, but using static instead of drupal_static(), xdebug disabled:
nothing: 0.24368596077 seconds
function no_op(): 0.654839992523 seconds
format_date() HEAD: 63.271586895 seconds
format_date() patch: 58.2883307934 seconds

Patch, but using $GLOBALS['drupal_static'] instead of drupal_static(), xdebug disabled:
nothing: 0.253867149353 seconds
function no_op(): 0.62436413765 seconds
format_date() HEAD: 62.4352011681 seconds
format_date() patch: 59.19520998 seconds

So the most we can get in real terms, is about a 6% improvement within format_date() (which is going to have a very tiny impact during the request), and this is only possible by using #537724: Consider $GLOBALS['drupal_static'] instead of drupal_static(), which needs a wider discussion.

Marking as Code Needs Work but this one's probably won't fix.

catch’s picture

FileSize
3.54 KB

forgot to attach script.

jrchamp’s picture

It's probably better if this one is "won't fix" (assuming that the performance difference is negligible). Identifying the locations where the statics need to be reset seems like it could be part of a larger problem.

catch’s picture

Status: Needs work » Closed (won't fix)

Let's won't fix for now.