Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:40007
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Date: Tue, 19 Aug 2008 09:23:48 -0700
Message-ID: <698DE66518E7CA45812BD18E807866CE01E77588@us-ex1.zend.net>
In-Reply-To: <48AA74C6.10507@zend.com>
Thread-Topic: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Thread-Index: AckBzIpyaP2MVYilS3iMoRjRfXdMDQAS1CDA
References: <200808170419.11153.arnaud.lb@gmail.com> <48A9B586.6000409@zend.com> <200808190116.04713.arnaud.lb@gmail.com> <48AA74C6.10507@zend.com>
To: "Dmitry Stogov" <dmitry@zend.com>,
	"Arnaud Le Blanc" <arnaud.lb@gmail.com>
Cc: "PHP Development" <internals@lists.php.net>,
	"Stas Malyshev" <stas@zend.com>
Subject: RE: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
From: andi@zend.com ("Andi Gutmans")

The original one which only caches the TLS index is bound to php.exe and
does not cross .dll boundaries (if I understand your question
correctly). The new one does and therefore will most likely crash on
Windows.

Andi


> -----Original Message-----
> From: Dmitry Stogov
> Sent: Tuesday, August 19, 2008 12:23 AM
> To: Arnaud Le Blanc
> Cc: PHP Development; Stas Malyshev; Andi Gutmans
> Subject: Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
>=20
> Hi Arnaud,
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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
>=20
> Anyway you did a great job. I would like to see this idea implemented
in
> HEAD. It is little bit late for 5.3 :(
>=20
> Thanks. Dmitry.
>=20
> >> 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