Optimize template_process()

Adding a static cache definitely makes sense. I'd be interested in the bench results.

- I'm wondering about the performance impact of using a potentially fairly long string as an array key

$key = implode(',', $paths) . ':' . implode(',', $suggestions) . ':' . $extension;

- Should probably use a comment to explain why it doesn't use drupal_static() ?

Also: isn't array_key_exists() notably slow ?

i don't see how yched's comments relate to the patch thats been posted. posted to wrong nid? anyway, this is a no brainer for such a heavily used function. and i'm usually against micro optimizations :)

Er, true, they were intended for #667030: Optimize drupal_discover_template(). Reposted them over there.

The patch here is OK.

I just found this independently and did microbenchmarks on the same patch as here, then searched for template_process() and found this.

Here's what I was going to post:

When viewing a listing of ten nodes, drupal_attributes() is called 383 times and takes up 6.7% of the request - 2/3rds of that is time spent in drupal_attributes() itself, just a foreach() and evaluating a ternary, but it's called often enough that it adds up.

We can cut 110 no-op calls to it by doing checks in template_process() instead of blindly passing through to drupal_attributes(). microbenchmarks on this with xdebug disabled, have it as twice as fast.

The test script just compares the equivalent of one line in the patch:

-  $variables['attributes'] = drupal_attributes($variables['attributes_array']);
+  $variables['attributes'] = $variables['attributes_array'] ? drupal_attributes($variables['attributes_array']) : '';

Calling the before/after on that 100,000 times generates the following results:

HEAD:
178ms

Patch:
81ms

If that works out about right, we'd save 1ms for every 200 no-op calls to drupal_attributes() that we can skip. That means this patch itself saves 0.5ms in the example request, but potentially more if template_process() gets called more often.

+  $variables['attributes'] = $variables['attributes_array'] ? drupal_attributes($variables['attributes_array']) : '';

Using empty() should be faster:

+  $variables['attributes'] = empty($variables['attributes_array']) ? '' : drupal_attributes($variables['attributes_array']);

http://codeigniter.com/forums/viewthread/135821/#669950

Committed #9 to CVS HEAD. We can follow-up with empty() clean-ups per #8 so I'm marking this 'needs work'. If we prefer a new issue, feel free to create a new issue and to mark this one 'fixed'. Thanks!

OK I did some benchmarks on bool vs. empty() and it looks like it's actually slower.

1 million calls:

function which does nothing:
0.601640939713 seconds

$foo ? $foo : '';
0.731494188309 seconds

!empty($foo) ? $foo : '';
0.818347930908 seconds

(in this case $foo was set to array(), but I got very similar results setting it to 'hello'. Since this needs more discussion / benchmarks, marking this as fixed and we can open a new issue if it's worth pursuing.

Catch did you try it with $foo not set?

Vars for all tests were:

$foo = array("bar" => "test");
$bar = array();

(went with it looking for an entry inside the array in case it made a difference)

run 1,000,000 times with the following results

Did nothing in 0.0865709781647 seconds
bar result: Array
Did normal way with foo-bar set in 0.336167812347 seconds
bar result: test
Did normal way with foo-bat NOT set in 0.526001930237 seconds
bar result: 
Did plain empty way with foo-bar set in 0.407722949982 seconds
bar result: test
Did plain empty way with foo-bat NOT set in 0.234389066696 seconds
bar result: 
Did not empty way with foo-bar set in 0.395099163055 seconds
bar result: test
Did not empty way with foo-bat NOT set in 0.307755947113 seconds

'normal way' in this sense meant

$bar = $foo['bar'] ? $foo['bar'] : '';

'plain empty way' meant:

 $bar = empty($foo['bar']) ? '' : $foo['bar'];

'not empty way' meant:

 $bar = !empty($foo['bar']) ? $foo['bar'] : '';

bar result was just checking for logic errors, making sure it got assigned the right value in each test. Ran each of the ways twice (over the million iterations of course); once on the correct 'bar' index and once on the incorrect 'bat' index. I ran this test multiple times and got heaps of different results as sometimes 'plain empty' was faster than 'not empty', other times they were reversed.

What was consistent is that the ones that used empty() were always a little slower than the 'normal' way when the array entry existed, this varied between 10-30% in the runs I checked. When the array entry didn't exist though, the 'empty ways' were both around 1.5x - 2x faster than the 'normal' way. Seems to me in functions that you know you'll be getting a lot of checks that turn up empty that it (empty()) may well be the way to go...

#684004: Minor optimisation in template_process_field() applies the same treatment to template_process_field().