On Wed, Mar 6, 2013 at 5:41 PM, Dmitry Stogov <dmi...@zend.com> wrote:
> Hi Nikita, > > few notes about the patch... > > - you may avoid estrndup() in zend_hash_current_key_zval_ex() for interned > strings. > Good idea, I'll do that. > > - I didn't completely get why did you change the "key" operand type from > IS_TMP_VAR to IS_VAR and how it affects performance > As I understood now you need to allocate new zval on each loop iteration > even for foreach over plain arrays. :( > Good point, I didn't consider the performance implication the type change would have. The intent behind that was to avoid copying the get_current_key zval into the temporary (and destroying it then), but I didn't consider how it affects normal arrays. This should be changed back to TMP_VAR. I wonder what would be a good way to avoid allocating a temporary zval for the key and freeing it again. Do you think it would be okay to pass &EX_T((opline+1)->result.var).tmp_var into ->get_current_key() so the value can be written directly into it without doing extra allocs/frees? Nikita
