On Tue, Mar 24, 2015 at 1:01 PM, Xinchen Hui <xinche...@zend.com> wrote: > Hey: > > On Tue, Mar 24, 2015 at 1:54 PM, Pierre Joye <pierre....@gmail.com> wrote: >> On Tue, Mar 24, 2015 at 12:35 PM, Xinchen Hui <xinche...@zend.com> wrote: >>> Hey: >>> >>> On Tue, Mar 24, 2015 at 1:31 PM, Pierre Joye <pierre....@gmail.com> wrote: >>>> hi! >>>> >>>> On Tue, Mar 24, 2015 at 5:41 AM, Dmitry Stogov <dmi...@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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
