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

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