Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:66497
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.219.50 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLtiOcFuxRXhpGXe5b9SpyVPB1_ixJ4t1GQ3J1bvBURMKA@mail.gmail.com>
References: <CAF+90c-E7n-WZcqNg28bX+G7zgbzhKoNCwR6hf++tfAgSzmtTw@mail.gmail.com>
	<CAF+90c-wSqeff_2oY8HeY+gDn-ZnwJeXKQ+cz-1W6xcEBjs+xw@mail.gmail.com>
	<CA+9eiLtiOcFuxRXhpGXe5b9SpyVPB1_ixJ4t1GQ3J1bvBURMKA@mail.gmail.com>
Date: Wed, 6 Mar 2013 18:16:27 +0100
Message-ID: <CAF+90c92AwjX7Niro_tT5vOv+vHeY-AmgfizL5f9i1FYhKTz2A@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: PHP internals <internals@lists.php.net>, Zeev Suraski <zeev@zend.com>, Andi Gutmans <andi@zend.com>
Content-Type: multipart/alternative; boundary=e89a8fb1ed06234ecb04d744c00d
Subject: Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach
From: nikita.ppv@gmail.com (Nikita Popov)

--e89a8fb1ed06234ecb04d744c00d
Content-Type: text/plain; charset=ISO-8859-1

On Wed, Mar 6, 2013 at 5:41 PM, Dmitry Stogov <dmitry@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

--e89a8fb1ed06234ecb04d744c00d--