Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:85435 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 88476 invoked from network); 24 Mar 2015 06:14:56 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 24 Mar 2015 06:14:56 -0000 Authentication-Results: pb1.pair.com smtp.mail=laruence@gmail.com; spf=pass; sender-id=pass Authentication-Results: pb1.pair.com header.from=laruence@gmail.com; sender-id=pass Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.48 as permitted sender) X-PHP-List-Original-Sender: laruence@gmail.com X-Host-Fingerprint: 209.85.215.48 mail-la0-f48.google.com Received: from [209.85.215.48] ([209.85.215.48:35208] helo=mail-la0-f48.google.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id DF/13-04797-CD001155 for ; Tue, 24 Mar 2015 01:14:53 -0500 Received: by laae1 with SMTP id e1so19392694laa.2 for ; Mon, 23 Mar 2015 23:14:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; bh=iNASUJWwy31tAzRiZLQvvcAJbzge/NzPJvod+5hfzFc=; b=GwgwTqcqSnfCtSgIq7lFmPDCJLZ1wbXtftmcbk/bnK23pqqw2ehuZvZyvYzPANbpDw IIg9fv59hwZLiR1vO8vTh7etodHBfMIFqrcRdRFOD7uD+mb/TC5lfe0UpQStRtz7LzDb qkyIfD5BXUlB32Whia27X6mNduIvCaNzrB0sIum5YCU4hKejWBTBE+3QWPO+qze6MNAK hQLJMdnO2W86t1UQW4m1BbWMnak/TSEBlDffN9wzB903ueMbkAhUvTiy1bau3gdfl9Eu UNVQtfC6YFO3twkveoCsbCh+Lb3h2VXuvozE2MdqcHTCrVMv+13EmQuZKCg9yyCwSxem TKjg== X-Received: by 10.112.210.201 with SMTP id mw9mr2308518lbc.13.1427177689862; Mon, 23 Mar 2015 23:14:49 -0700 (PDT) MIME-Version: 1.0 Sender: laruence@gmail.com Received: by 10.114.203.39 with HTTP; Mon, 23 Mar 2015 23:14:29 -0700 (PDT) In-Reply-To: References: Date: Tue, 24 Mar 2015 14:14:29 +0800 X-Google-Sender-Auth: wmJkfZLA5WFREaSZPiQTRuNpA2Q Message-ID: To: Pierre Joye Cc: Dmitry Stogov , Nikita Popov , Joe Watkins , PHP Internals Content-Type: text/plain; charset=UTF-8 Subject: Re: [PHP-DEV] Use "ropes" for string concatenation From: xinchen.h@zend.com (Xinchen Hui) Hey: On Tue, Mar 24, 2015 at 2:04 PM, Pierre Joye wrote: > On Tue, Mar 24, 2015 at 1:01 PM, Xinchen Hui wrote: >> Hey: >> >> On Tue, Mar 24, 2015 at 1:54 PM, Pierre Joye wrote: >>> On Tue, Mar 24, 2015 at 12:35 PM, Xinchen Hui wrote: >>>> Hey: >>>> >>>> On Tue, Mar 24, 2015 at 1:31 PM, Pierre Joye wrote: >>>>> hi! >>>>> >>>>> On Tue, Mar 24, 2015 at 5:41 AM, Dmitry Stogov 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 In that case, yeah. you might be right. but from my opinion, simpler always means easier for maintaining.... anyway, I hope this could be merged to 7.0(the second branch) :) thanks > >> and regarding to the object overriden concat..... >> >> I doubt it is not used at all :) > > -- > Pierre > > @pierrejoye | http://www.libgd.org -- Xinchen Hui @Laruence http://www.laruence.com/