Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:64137
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain zend.com does not designate 209.85.216.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <201212031518.32545.yoram.b@zend.com>
References: <CA+9eiLsojD5fPCbrYqqcLyYQ30jSU0yqAaPy=U_rzEG+Zd0vQQ@mail.gmail.com>
	<CA+9eiLu6neE9Wf3qo+rDFm5cKew-WMT65Drxz9YcKPrf14dOSw@mail.gmail.com>
	<CA+9eiLtgz+S=-aTWJXeUA+B4VZjpVEu=hWiJSpYZnRAsAeRPdA@mail.gmail.com>
	<201212031518.32545.yoram.b@zend.com>
Date: Mon, 3 Dec 2012 16:26:18 +0300
Message-ID: <CA+9eiLtnudJzS8AVEprDWaVYJZYJfQy1+1=orFjf9Qwn8Mu8kg@mail.gmail.com>
To: yoram bar haim <yoram.b@zend.com>
Cc: Rasmus Lerdorf <rasmus@lerdorf.com>, Derick <derick@php.net>, jpauli@php.net, 
	PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=20cf3033463fc226ac04cff2b1e8
Subject: Re: [PHP-DEV] execute_data->Ts removing
From: dmitry@zend.com (Dmitry Stogov)

--20cf3033463fc226ac04cff2b1e8
Content-Type: text/plain; charset=UTF-8

Hi Yoram,

Yeas, I checked memory reads on a release PHP built (with -O2 optimization
level).

The idea is very simple. Before the patch each access to TMP was
implemented as:

execute_data->Ts + offset

GCC with -O2 kept execute_data in register but it needed to read
execute_data->Ts anyway and then add the offset

Now we just add the offset to execute_data directly.

Also, the patch I published today is not compatible with 64-bit systems.
I already fixed it, and the fix is minimal.

Thanks. Dmitry.



On Mon, Dec 3, 2012 at 5:18 PM, yoram bar haim <yoram.b@zend.com> wrote:

> Hi Dmitry.
> is this improvemnt (intruction reduce and memory read reduce) preserved
> under
> "strong" compiler optimization (-O2, -O3) ?
> is it something that the compiler can not automatically optimize ?
> anyhow, the patch seems impressive...
>
> On Monday, December 03, 2012 11:35:52 AM Dmitry Stogov wrote:
> > The new proposed patch: http://pastebin.com/pj5fQTfN
> >
> > Now both execute_data->Ts and execute_data->CVs are removed and
> > corresponding temporary and compiled variables accessed using
> > "execute_data" as the base pointer. Temporary variables allocate directly
> > before the "execute_data" in reverse order and compiled variables right
> > after. So they can be accessed without any additional computations. The
> > patch reduces the number of executed instructions and number of memory
> > reads (about 8% less).
> >
> > I'm going to commit the patch on Tuesday.
> >
> > Thanks. Dmitry.
> >
> > On Mon, Dec 3, 2012 at 9:47 AM, Dmitry Stogov <dmitry@zend.com> wrote:
> > > Hi Rasmus,
> > >
> > > I'm glad the patch is not a big problem for APC.
> > > BTW: I decided not to commit the patch as is, and implement the similar
> > > optimization for CVs first.
> > > Otherwise I may need to change the way TMP_VAR accessed twice.
> > > I'll probably send the patch for review later today.
> > >
> > > Thanks. Dmitry.
> > >
> > > On Fri, Nov 30, 2012 at 11:48 PM, Rasmus Lerdorf
> <rasmus@lerdorf.com>wrote:
> > >> On 11/30/2012 09:15 AM, Dmitry Stogov wrote:
> > >> > Hi,
> > >> >
> > >> > The NEWS and UPGRADING explains the details.
> > >> >
> > >> > http://pastebin.com/VC71Y8LV
> > >> >
> > >> > The patch is big, but actually quite simple.
> > >> > I'm going to commit it on Monday or Tuesday (if no objections).
> > >> >
> > >> > I'm going to look into the similar optimization for CVs, but it's
> > >> > going
> > >>
> > >> to
> > >>
> > >> > be a bit more difficult.
> > >>
> > >> Looks good to me. I'll commit this change to APC when you commit it:
> > >>
> > >> Index: apc_zend.c
> > >> ===================================================================
> > >> --- apc_zend.c  (revision 328577)
> > >> +++ apc_zend.c  (working copy)
> > >> @@ -48,7 +48,11 @@
> > >>
> > >>  static opcode_handler_t *apc_original_opcode_handlers;
> > >>  static opcode_handler_t
> apc_opcode_handlers[APC_OPCODE_HANDLER_COUNT];
> > >>
> > >> +#if PHP_MAJOR_VERSION >= 6 || PHP_MAJOR_VERSION == 5 &&
> > >> PHP_MINOR_VERSION >= 5
> > >> +#define APC_EX_T(offset)
>  (*EX_TMP_VAR(execute_data,
> > >> offset))
> > >> +#else
> > >>
> > >>  #define APC_EX_T(offset)                    (*(temp_variable
> > >>
> > >> *)((char*)execute_data->Ts + offset))
> > >> +#endif
> > >>
> > >> And there are a couple of extensions that are going to need similar
> > >> changes because of this. pecl/optimizer, pecl/inclued, pecl/xhprof,
> > >> pecl/operator and xdebug from a quick check.
> > >>
> > >> -Rasmus
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--20cf3033463fc226ac04cff2b1e8--