According to the latest benchmarks, the theme system, and the drupal_render pipeline in particular, is one of our larger time sinks. drupal_render() is intensely recursive, and with drupal_render_children() is double-recursive. PHP stack calls are not particularly fast. So let's see if we can save ourselves a few stack calls in drupal_render(), which is along pretty much every critical path now.
The attached patch removes the call to drupal_render_children() from drupal_render() and inlines the same behavior. There should be zero functionality change, but we do save one function call for every time we need to render a child element. I did not remove drupal_render_children() because it's still called from a ton of theme functions, but at least this will reduce the recursion depth.
With APC enabled using ab -n200 -c1
on node 1, Before:
Requests per second: 10.72 [#/sec] (mean)
Time per request: 93.251 [ms] (mean)
Time per request: 93.251 [ms] (mean, across all concurrent requests)
And After:
Requests per second: 11.07 [#/sec] (mean)
Time per request: 90.373 [ms] (mean)
Time per request: 90.373 [ms] (mean, across all concurrent requests)
Not a massive difference, but every little bit helps. It should also be a bigger difference on sites with more complex node bodies, or deep forms like CCK forms, because those have deeper recursion in the rendering pipeline.
Comment | File | Size | Author |
---|---|---|---|
#5 | inline-children.patch | 900 bytes | Crell |
inline-children.patch | 898 bytes | Crell | |
Comments
Comment #1
Crell CreditAttribution: Crell commentedGah, forgot to tag.
Comment #3
Crell CreditAttribution: Crell commentedSigh. And this time hopefully without stupid bugs I have no business writing.
Before:
After:
Comment #4
catchI profiled locally. On a very simple page, drupal_render_children() is 0.3% of page execution time. That time is (obviously) removed by this patch, but not back into drupal_render(). It's a tiny optimization, but it's a good one, and still very readable.
Comment #5
Crell CreditAttribution: Crell commentedAnd this time with a patch attached, too!
Comment #6
catchVery small optimization but it's still quite readable, so a no-brainer.
Comment #7
Crell CreditAttribution: Crell commentedTagging. A < 1 KB patch should not still be sitting in the queue...
Comment #8
Dries CreditAttribution: Dries commentedLooks good to me, although it is somewhat of a micro-optimization. Committed to CVS HEAD. Thanks!