Optimize array_fill_keys() #20347
Optimize array_fill_keys() #20347
Conversation
Move the refcount update outside of the loop.
For the following benchmark:
```php
$r = range(0, 1000);
$v = new stdClass();
for ($i = 0; $i < 100000; $i++) {
array_fill_keys($r, $v);
}
```
On an i7-4790:
```
Benchmark 1: ./sapi/cli/php_old ../x.php
Time (mean ± σ): 507.5 ms ± 4.8 ms [User: 505.1 ms, System: 1.2 ms]
Range (min … max): 501.2 ms … 518.4 ms 10 runs
Benchmark 2: ./sapi/cli/php ../x.php
Time (mean ± σ): 479.8 ms ± 3.1 ms [User: 476.8 ms, System: 1.8 ms]
Range (min … max): 475.0 ms … 486.7 ms 10 runs
Summary
./sapi/cli/php ../x.php ran
1.06 ± 0.01 times faster than ./sapi/cli/php_old ../x.php
```
On an i7-1185G7:
```
Benchmark 1: ./sapi/cli/php x.php
Time (mean ± σ): 343.9 ms ± 3.1 ms [User: 341.1 ms, System: 2.3 ms]
Range (min … max): 337.9 ms … 347.8 ms 10 runs
Benchmark 2: ./sapi/cli/php_old x.php
Time (mean ± σ): 357.8 ms ± 2.3 ms [User: 355.7 ms, System: 1.6 ms]
Range (min … max): 355.0 ms … 362.6 ms 10 runs
Summary
./sapi/cli/php x.php ran
1.04 ± 0.01 times faster than ./sapi/cli/php_old x.php
```
| if (Z_REFCOUNTED_P(val)) { | ||
| GC_DELREF_EX(Z_COUNTED_P(val), keys_count - zend_hash_num_elements(Z_ARRVAL_P(return_value))); | ||
| } |
There was a problem hiding this comment.
This is necessary in case of exception?
There was a problem hiding this comment.
FWIU it's needed for duplicate keys. That said, I'm not sure why we can't just add the refs at the end of the function, given we're already holding onto a strong reference to val through the stack, so it can't be freed in a __toString() method.
There was a problem hiding this comment.
Ilija is right; duplicate keys will cause the refcounter to be too large.
I wasn't sure whether a bailout could cause issues in this case, which is why I did the addrefs beforehand. But I suppose this can't cause issues when I look at it now. I'll adjust.
There was a problem hiding this comment.
Right. In case of bailout, the return value hasn't escaped yet, so there shouldn't be a RC violation.
3 participants
Move the refcount update outside of the loop.
For the following benchmark:
On an i7-4790:
On an i7-1185G7: