Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:85434
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.45 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CABBJUpe+CgQVMJeh7pa93BBtCXkP-4oXDHVR0xvd-D9FpeRyRQ@mail.gmail.com>
References: <CA+9eiLsAQPTkVuR=D0wXMFXhBNM8DQzGq0DRXzSuL+hjPsUpgA@mail.gmail.com>
	<CAEZPtU7yj6cYpBd_1hfZkr5F9S2Jw_JhuGKK=M=w5qS3o7uTqw@mail.gmail.com>
	<CABBJUpe4zmwzK3PJ+6DhAvwjLr_pfAcNdkouChNVV-OaRjWpgw@mail.gmail.com>
	<CAEZPtU4h2RnGJ7GuNU4uvD8dZz97Eo2k8OiOQYVF3kz-_VdQAQ@mail.gmail.com>
	<CABBJUpe+CgQVMJeh7pa93BBtCXkP-4oXDHVR0xvd-D9FpeRyRQ@mail.gmail.com>
Date: Tue, 24 Mar 2015 13:04:41 +0700
Message-ID: <CAEZPtU6s4_-dJJqeFgjS3nsVqB+K3zy=g-ge74en7tG7Ga0tBw@mail.gmail.com>
To: Xinchen Hui <xinchen.h@zend.com>
Cc: Dmitry Stogov <dmitry@zend.com>, Nikita Popov <nikita.ppv@gmail.com>, 
	Joe Watkins <pthreads@pthreads.org>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Use "ropes" for string concatenation
From: pierre.php@gmail.com (Pierre Joye)

On Tue, Mar 24, 2015 at 1:01 PM, Xinchen Hui <xinchen.h@zend.com> wrote:
> Hey:
>
> On Tue, Mar 24, 2015 at 1:54 PM, Pierre Joye <pierre.php@gmail.com> wrote:
>> On Tue, Mar 24, 2015 at 12:35 PM, Xinchen Hui <xinchen.h@zend.com> wrote:
>>> Hey:
>>>
>>> On Tue, Mar 24, 2015 at 1:31 PM, Pierre Joye <pierre.php@gmail.com> wrote:
>>>> hi!
>>>>
>>>> On Tue, Mar 24, 2015 at 5:41 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>>>>> Hi,
>>>>>
>>>>> Recently, Xinchen and me worked on optimization that eliminates useless
>>>>> reallocations and copying during string concatenation (ZEND_ADD_STRING and
>>>>> family + ZEND_CONCAT).
>>>>>
>>>>> The idea comes from ropes, but adopted especially for our needs.
>>>>> Rope is popular data structure in languages with immutable strings.
>>>>> http://en.wikipedia.org/wiki/Rope_%28data_structure%29
>>>>>
>>>>> We don't try to use ropes everywhere in the engine (at least it's too later
>>>>> for 7.0), only for concatenation.
>>>>>
>>>>> The first branch uses ropes only instead of ZEND_ADD_STRING and family.
>>>>> This must be safe. The only problem is possible memory leaks on exception
>>>>> (but we already have this problem anyway). The simplest way to understand
>>>>> the patch - read code for new opcodes in zend_vm_def.h.
>>>>>
>>>>> https://github.com/php/php-src/pull/1194/files
>>>>>
>>>>> The second branch in addition uses ropes for series of ZEND_CONCAT. It
>>>>> disables calls to do_operation(ZEND_CONCAT) handler of custom internal
>>>>> classes.
>>>>>
>>>>> https://github.com/php/php-src/pull/1195/files
>>>>>
>>>>> Both make slight speed improvement (first +0.3%, second +0.6% on wordpress
>>>>> home page).
>>>>>
>>>>> We don't currently use ability to override CONCAT behavior in internal
>>>>> classes, and I'm not sure if it may be useful at all. (For example Lua
>>>>> doesn't provide concat meta-method). May be remove it?
>>>>>
>>>>> Thoughts and comments are welcome.
>>>>
>>>> Great work! :)
>>>>
>>>> Do you expect similar gain for other ops?
>>>>
>>>> I wonder if it would not be better to target 7.1 for that,  adding it
>>>> for other string operations, in one go. Most of the current patch is
>>>> self contained, it adds quite some complexity to the engine for these
>>>> operations only and it is not a small change at this stage (post
>>>> features freeze). It sounds like a possible maintenance pain. Taking
>>> Actually, it's not.
>>>
>>> previously we have ADD_STRING/CHAR/VAR and CONCAT
>>>
>>> the 2nd branch cleanup these, and now we only deal with one type concat_list. :)
>>
>> Right, it simplifies part of the existing implementation and add some
>> complexities to other. It only adds this OP and keep other with the
>> "old" implementation, that's actually the only part where I am not
>> totally convinced. It may make sense to do it all at once, if that's
>> the long term plan, easier to maintain.
>>
>> But if we do prefer to add this now, then the sooner the better, it
>> may have some hard to catch bugs. Now, we have one issue, we cannot
>> have a RFC (deadline behind us) and this is still a rather big change
>> :/
> from user land.  this won't change anything..

Nothing to do with userland or not but code stabilization

> and regarding to the object overriden concat.....
>
> I doubt it is not used at all :)

-- 
Pierre

@pierrejoye | http://www.libgd.org