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?


Reply via email to