r67869 MediaWiki - Code Review archive

You might want to think about flipping that array. in_array() is O(N), and isset($a[$k]) is O(1). The setup time for an associative array is slightly longer, but IIRC previous profiling in a similar situation indicated that it makes sense if you have to call the function more than about 10 times.

0 10:46:08 /var/www/git-trunk/phase3$ time echo '<?php $x = array( "a", "b", "c", "d", "e", "f", "g", "h", "i", "j" ); for ( $i = 0; $i < 1000000; $i++ ) { in_array( "g", $x ); in_array( "q", $x ); }' | php

real	0m2.860s
user	0m2.728s
sys	0m0.024s
0 10:46:12 /var/www/git-trunk/phase3$ time echo '<?php $x = array( "a" => "", "b" => "", "c" => "", "d" => "", "e" => "", "f" => "", "g" => "", "h" => "", "i" => "", "j" => "" ); for ( $i = 0; $i < 1000000; $i++ ) { isset( $x["g"] ); isset( $x["q"] ); }' | php

real	0m0.398s
user	0m0.380s
sys	0m0.008s

That says to me that an associative array would be much faster, but we're talking about a microsecond per call either way. The function is only going to be called once per attribute in the output at most, and pages typically don't have more than a few thousand attributes. Granted, this pattern occurs a number of times in the file, but is this really a useful optimization except in very hot code paths? I'll do it if you want, but it does make the code slightly uglier.

The difference it makes depends on the array size. For an array with 22 elements instead of 10, like $boolAttribs, I get a difference of 2.5us per call. About 0.8us of that is actually the function call overhead, judging by the array_key_exists() timings. See Sanitizer::removeHTMLtags() for an example of how ugly it would be. Html::expandAttributes() is probably the hottest function in the Html module, but if you don't want to do it, that's OK with me.