Re: [PATCH] ZTS as fast as non-ZTS

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. > > 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). > > 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