Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:85453
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.212.178 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLv0pakhXf9NhpiF+TkPgKd52ATH=dUdrpu1fAM2GAPSbw@mail.gmail.com>
References: <CA+9eiLsAQPTkVuR=D0wXMFXhBNM8DQzGq0DRXzSuL+hjPsUpgA@mail.gmail.com>
	<CAF+90c9Kw0iGxEZkpwbfNV+uZKrnpJ32gz9g257w00G1-rC1JQ@mail.gmail.com>
	<CA+9eiLt2tUVJ40_YTVq5TgUWX0a0URvmYwFQU=33nxManhXsng@mail.gmail.com>
	<CAF+90c9B7seGdPdn-7=mxinFVHXU_rvdK_EDouW+A01HWjkWFw@mail.gmail.com>
	<CA+9eiLv0pakhXf9NhpiF+TkPgKd52ATH=dUdrpu1fAM2GAPSbw@mail.gmail.com>
Date: Tue, 24 Mar 2015 20:45:14 +0100
Message-ID: <CAF+90c_p5NEt1qRnNSsHwVmLM0JmLrKO_WNOeLanK7yYuy-WgA@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: Xinchen Hui <xinchen.h@zend.com>, Joe Watkins <pthreads@pthreads.org>, 
	PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7bb042be7857af05120e0548
Subject: Re: Use "ropes" for string concatenation
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7bb042be7857af05120e0548
Content-Type: text/plain; charset=UTF-8

On Tue, Mar 24, 2015 at 1:08 PM, Dmitry Stogov <dmitry@zend.com> wrote:

>
>
> On Tue, Mar 24, 2015 at 2:51 PM, Nikita Popov <nikita.ppv@gmail.com>
> wrote:
>
>> On Tue, Mar 24, 2015 at 10:28 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>>
>>> I thought about something like this :)
>>> In my opinion UString is really not a proper way to implement Unicoide,
>>> but I agree not break anything in current stage.
>>> Anyway, please review the first PR (in my opinion it is safe to commit),
>>> but you may find some other issues.
>>>
>>> Thanks. Dmitry.
>>>
>>
>> First PR looks okay to me. One question: Why the separate INIT and ADD
>> opcodes? They seem pretty much the same, just one using a hardcoded 0
>> instead of ex_val.
>>
>
> Oh. It's historical, at first we allocated rope on heap. I'll check if
> INIT_ROPE is still necessary.
> Most probably we still need it, to know the size of rope vector.
>
>
>> Regarding exception-safety - is the problem that doing an EG(exception)
>> check and releasing the rope is too expensive?
>>
>
> No. It's a common problem. Because exception may be thrown in some other
> opcode and we don't clean IS_VAR/IS_TMP_VAR zvals.
> For example the following script leaks independently from the patch
>
> <?php
> function foo() {
>   throw new Exception();
> }
> try {
>         $a = "a";
>         $a = $a . $a . $a . foo() . $a . "\n";
> } catch (Exception $e) {
> }
> ?>
>

Ah yes, I forgot about this issue. However, shouldn't we at least properly
release memory if the exception occurs in the opcode?

Nikita

--047d7bb042be7857af05120e0548--