Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Drupal has about 118 calls to is_array() in core. Many could be short-circuited and never called by checking if the variable is set. More could be eliminated if arguments were either arrays or were not.
Making these changes should enhance performance.
See http://www.php.lt/benchmark/phpbench.php.
is_array() is 3 to 7 times slower than isset().
Comment | File | Size | Author |
---|---|---|---|
#13 | chameleonsecondaryisset.patch | 825 bytes | Chris Johnson |
#6 | is_array2.patch | 16.16 KB | Chris Johnson |
#1 | is_array.patch | 14.03 KB | Chris Johnson |
Comments
Comment #1
Chris Johnson CreditAttribution: Chris Johnson commentedHere's a patch which speeds up all the is_array() calls where it was obvious to me that a short-circuit could be built for it.
Some is_array() calls were left untouched as they were ok. A few others were left untouched because I was unsure of all the circumstances that they might be called in.
Comment #2
Chris Johnson CreditAttribution: Chris Johnson commentedComment #3
shouchen CreditAttribution: shouchen commentedComment #4
chx CreditAttribution: chx commentedif ($terms['tags']) { // fixme
I doubt this is needed :)Also, in pushbutton page.tpl.php the is_array is not needed. In bluemarine it's
if ($primary_links)
this needs a look into menu_primary_links about its return values. That returns eitherNULL
or anarray
. This tells us that both core themes needs anisset
and the same for secondary.Comment #5
chx CreditAttribution: chx commentedAlso, in form_render the if is_array check is redundant. The whole function presumes it's an array.
Comment #6
Chris Johnson CreditAttribution: Chris Johnson commentedRevised patch should handle all issues raised.
Comment #7
Chris Johnson CreditAttribution: Chris Johnson commentedWoops, accidentally changed the title. (and forgot to update the status).
Comment #8
chx CreditAttribution: chx commentednice.
Comment #9
Dries CreditAttribution: Dries commentedDid you do some benchmarks? Any benchmark results?
Comment #10
Chris Johnson CreditAttribution: Chris Johnson commentedDries: No, I haven't done any system-wide benchmarks on this. But on a head-to-head comparison of "isset($i) && is_array($i)" versus "is_array($i)" alone, where $i is not set, the former is 2 to 7 times faster.
Some ab benchmarks with and without the patch probably would seem the most direct way to do it.
Volunteers? I'm rather busy. :-)
Comment #11
Dries CreditAttribution: Dries commentedCommitted to HEAD. Thanks.
This patch had 2 parse errors; please try to be a tad more careful when marking this as 'ready to be committed'. Thanks.
Comment #12
Tobias Maier CreditAttribution: Tobias Maier commentedyou forgot one isset() at chameleon.theme
at
if (($secondary_links)) {
it has to be
if (isset($secondary_links)) {
Comment #13
Chris Johnson CreditAttribution: Chris Johnson commentedOdd. The parens made it for the function call, but not the function name. Anyway, here's a patch that fixes that.
Comment #14
Cvbge CreditAttribution: Cvbge commentedThe patch probably won't apply because of changed $id header.
Comment #15
Cvbge CreditAttribution: Cvbge commentedprobably won't -> might not
Comment #16
Gábor HojtsyEhem, this patch made the revisions update broken (the update never runs, the update process hands on forever). I offered a patch as part of my patch pack for the update process, which fixes several critical issues now.
Comment #17
Dries CreditAttribution: Dries commentedCommitted to HEAD. Thanks.
Comment #18
(not verified) CreditAttribution: commented