Improvements to array.c code base

9 years ago by Benjamin Coutu — view source

unread

Hi,

Regarding ARRAY_SLICE(..., preserve_keys=false) my whole point was that the check for non-numeric key inputs is not necessary if preserve_keys is false.
It should basically be analogue to array_values(), but of course respecting the offset and length boundaries.

One question regarding FAST_ZPP: Why is it not used more often, especially in the satndard libraries? Is there some particular drawback? I'd like to understand.

Thanks,

Ben

========== Original ==========
From: Xinchen Hui xinchen.h@zend.com
To: Benjamin Coutu ben.coutu@zeyos.com
Date: Tue, 20 Jan 2015 09:53:38 +0100
Subject: [PHP-DEV] Re: Improvements to array.c code base

Hey:

Hi Dmitry,

I was doing some code review of ext/standard/array.c and have recognized some potential for a few performance improvements:

=== ARRAY_SLICE(..., preserve_keys=false) ===

array_slice() can always construct a packed array if preserve_keys is false, restricting it to inputs with packed flag does not make much sense.
Removing the check for packed inputs on line 2376 would improve performance if used on non-packed inputs with the default preserve_keys=false.
This is check for non-numeric key inputs

Furthermore, ZEND_HASH_FOREACH_VAL should be used instead of ZEND_HASH_FOREACH_NUM_KEY_VAL on line 2379.
yeah, I will fix it.
It also think range() should use FAST_ZPP as it is a basic language feature (other languages even have extra operators for it, e.g. [0..10])

=== RANGE(...) ===

range() always returns a numerically indexed array [0..count-1]. The resulting array therefore should be constructed as a packed array (ZEND_HASH_FILL_PACKED+ZEND_HASH_FILL_ADD instead of zend_hash_next_index_insert_new).

=== ARRAY_FILL(start_key=0, ...)

Just like with range(), array_fill() always returns a numerically indexed array [0..count-1] if start_key is 0. In this special but very common case the resulting array can be constructed as a packed array (ZEND_HASH_FILL_PACKED+ZEND_HASH_FILL_ADD instead of zend_hash_next_index_insert_new).
Another common case is for start_key to be 1. One could refine the proposed packed-array branch to just set the first bucket to undefined in this case.

=== COUNT(...) ===

count() is so ubiquitous! Giving it an opcode and making it part of the VM seams reasonable.

Please let me know your thoughts.
hmm, we currently do optimization based on profile against some real
life apps, like wordpress.

range and array_fill etc doesn't used very common.. so I didn't look into it..

thanks for the advise, I will take a look .

thanks

Cheers,

Ben

--

Benjamin Coutu
Zeyon Technologies Inc.
http://www.zeyos.com