Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:85430
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.46 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLsAQPTkVuR=D0wXMFXhBNM8DQzGq0DRXzSuL+hjPsUpgA@mail.gmail.com>
References: <CA+9eiLsAQPTkVuR=D0wXMFXhBNM8DQzGq0DRXzSuL+hjPsUpgA@mail.gmail.com>
Date: Tue, 24 Mar 2015 12:31:53 +0700
Message-ID: <CAEZPtU7yj6cYpBd_1hfZkr5F9S2Jw_JhuGKK=M=w5qS3o7uTqw@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: Xinchen Hui <xinchen.h@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)

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
more time to actually experiment and implement this idea sounds safer
to me (read: we have a bit less than a year to valid it then).

Cheers,
-- 
Pierre

@pierrejoye | http://www.libgd.org