Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:40046
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.134.190 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=from:to:subject:date:user-agent:cc:references:in-reply-to
         :mime-version:content-type:content-transfer-encoding
         :content-disposition:message-id;
        b=sn6iRAhlkZBq4IXYiHgXH2aGboQHCTogOKM3D6Rxhp3Nq44AeCDruFkLb2yM3eckcR
         XjGG3c2Ml90FuBOcfJH21u7zdvpYnsPEaKAfqhTBb1PB96Fx/U8ZFEuT75CGHIrc5CGh
         4MNS7HTuGt3H+ivEQ9V/16safH/NCtd0vESFM=
To: Dmitry Stogov <dmitry@zend.com>
Date: Thu, 21 Aug 2008 14:30:31 +0200
User-Agent: KMail/1.10.0 (Linux/2.6.26-noch; KDE/4.1.0; i686; ; )
Cc: PHP Development <internals@lists.php.net>,
 Stanislav Malyshev <stas@zend.com>,
 Andi Gutmans <andi@zend.com>
References: <200808170419.11153.arnaud.lb@gmail.com> <200808200747.07817.arnaud.lb@gmail.com> <48AD1B28.6020709@zend.com>
In-Reply-To: <48AD1B28.6020709@zend.com>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-ID: <200808211430.32861.arnaud.lb@gmail.com>
Subject: Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
From: arnaud.lb@gmail.com (Arnaud Le Blanc)

Hi,

On Thursday 21 August 2008 09:37:12 Dmitry Stogov wrote:
> 
> Arnaud Le Blanc wrote:
> > Hi,
> > 
> > On Tuesday 19 August 2008 09:22:46 Dmitry Stogov wrote:
> >> Hi Arnaud,
> >>
> >> Arnaud Le Blanc wrote:
> >>> Hi,
> >>>
> >>> On Monday 18 August 2008 19:46:46 Dmitry Stogov wrote:
> >>>> Hi Arnaud,
> >>>>
> >>>> The patch looks very interesting.
> >>>> I think it may be committed to the HEAD in the nearest future.
> >>>> I don't have time to look into all details in the moment.
> >>>>
> >>>> Could you explain why --with-tsrm-full-__thread-tls doesn't work with 
> >>>> dlopen() however --with-tsrm-__thread-tls does?
> >>> That's due to the way TLS works internally. Actually I need further 
> > reading on 
> >>> that.
> >> I don't see a big difference between --with-tsrm-full-__thread-tls and 
> >> --with-tsrm-__thread-tls from TLS point of view (may be I miss it), so I 
> >> don't understand why one works and the other doesn't.
> > 
> > Badly both was not expected to work with dlopen()  :( 
> > http://marc.info/?l=php-internals&m=121912220705964&w=2
> > 
> > It works when using an other TLS model (which requires PIC).
> > This model is less efficient but it still improves performance. PIC-patched 
is 
> > faster than non-PIC-unpatched, and the improvement is even greater against 
> > PIC-unpatched.
> > 
> 
> This is the thing I was afraid.
> 
> >> The patch looks more difficult for me than it should. I would prefer to 
> >> have only --with-tsrm-full-__thread-tls, if it works, as the patch would 
> >> be simple and PHP faster.
> >>
> >> Another simple solution, which you probably already tested, is to use 
> >> only global __thread-ed tsrm_ls (and don't pass/fetch it), however, 
> >> access thread-globals in the same way: 
> >> ((*type)tsrm_ls[global_module_id])->global_fileld
> > 
> > Yes, I tested and it given 4.7s on bench.php (vs 3.8s with the current 
patch).
> >
> 
> Does this model have the same issues with dlopen()?
> (We have only one __thread-ed variable).

Yes and no. By reading the loader/linker code in the glibc I seen that it 
reserves a number of memory especially for the dlopen() case. This number of 
memory it too small for --with-tsrm-full-__thread-tls, but several times 
larger than needed for --with-tsrm-__thread-tls. So the memory found by 
dlopen() is not random and I think we can expect that to work on Linux. 
However things are different on Windows and FreeBSD. I will post a RFC about 
that to clarify things.

> 
> Thanks. Dmitry.
> 
> >> Anyway you did a great job. I would like to see this idea implemented in 
> >> HEAD. It is little bit late for 5.3 :(
> >>
> >> Thanks. Dmitry.
> >>
> >>>> Did you test the patch with DSO extensions?
> >>> I will, but I guess that will be behaves like another shared library 
> >>> dlopen()ed by Apache.
> >>>
> >>>> It would be interesting to try the same idea on Windows with VC.
> >>> I will try too.
> >>>
> >>>> Thanks. Dmitry.
> >>>>
> >>>> Arnaud Le Blanc wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Currently the way globals work forces to pass a thread-local-storage 
> >>> pointer 
> >>>>> across function calls, which involves some overhead. Also, not all 
> >>> functions 
> >>>>> get the pointer as argument and need to use TSRMLS_FETCH(), which is 
> > slow. 
> >>> For 
> >>>>> instance emalloc() involves a TSRMLS_FETCH(). An other overhead is 
> >>> accessing 
> >>>>> globals, using multiple pointers in different locations.
> >>>>>
> >>>>> The following patch caches each global address in a native TLS 
variable 
> > so 
> >>>>> that accessing a global is as simple as global_name->member. This 
> > removes 
> >>> the 
> >>>>> requirement of passing the tls pointer across function calls, so that 
> > the 
> >>> two 
> >>>>> major overheads of ZTS builds are avoided.
> >>>>>
> >>>>> Globals can optionally be declared statically, which speeds up things 
a 
> >>> bit.
> >>>>> Results in bench.php:
> >>>>> non-ZTS:						3.7s
> >>>>> ZTS unpatched:					5.2s
> >>>>> ZTS patched:					4.0s
> >>>>> ZTS patched and static globals:	3.8s
> >>>>>
> >>>>> The patch introduces two new macros: TSRMG_D() (declare) and 
TSRMG_DH() 
> >>>>> (declare, for headers) to declare globals, instead of the current 
> >>> "ts_rsrc_id 
> >>>>> foo_global_id". These macros declare the global id, plus the __thread 
> >>> pointer 
> >>>>> to the global storage.
> >>>>>
> >>>>> ts_allocate_id now takes one more callback function as argument to 
bind 
> >>> the 
> >>>>> global pointer to its storage. This callback is declared in TSRMG_D[H]
> > ().
> >>>>> As all TSRMLS_* macros now does nothing, it is needed to call 
> >>> ts_resource(0) 
> >>>>> explicitly at least one time in each thread to initialize its storage. 
A 
> >>> new 
> >>>>> TSRMLS_INIT() macro as been added for this purpose.
> >>>>>
> >>>>> All this is disabled by default. --with-tsrm-__thread-tls enables the 
> >>> features 
> >>>>> of the patch, and --with-tsrm-full-__thread-tls enables static 
> > declaration 
> >>> of 
> >>>>> globals.
> >>>>>
> >>>>> It as been tested on Linux compiled with --disable-all in CLI and a 
bit 
> > in 
> >>>>> Apache2 with the worker MPM. Known issues: 
> >>>>> - Declaring globals statically (--with-tsrm-full-__thread-tls) causes 
> >>> troubles 
> >>>>> to dlopen(), actually Apache wont load the module at runtime (it works 
> >>> with 
> >>>>> just --with-tsrm-__thread-tls).
> >>>>> - The patch assumes that all resources are ts_allocate_id()'ed before 
> > any 
> >>>>> other thread calls ts_allocate_id or ts_resource_ex(), which is 
possibly 
> >>> not 
> >>>>> the case.
> >>>>>
> >>>>> The patch needs some tweaks and does not pretend to be included in any 
> >>> branch, 
> >>>>> but I would like to have some comments on it.
> >>>>>
> >>>>> The patch: http://arnaud.lb.s3.amazonaws.com/__thread-tls.patch
> >>>>>
> >>>>> Regards,
> >>>>>
> >>>>> Arnaud
> >>>>>
> >>>>>
> >>> Regards,
> >>>
> >>> Arnaud
> > 
> > Regards,
> > 
> > Arnaud
>