Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:46267
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.44 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :content-type:content-transfer-encoding;
        b=WrR6QUlgYrh2TXDqjIMGUpTkmV7QtbaoZi61OtndJim9MvwFfN+nb5/Ij89+0aMs6S
         2uTc9BebddAPDQ8maGX0XaLYP3MPCvW+3JF3o1j9I/9ZLEYUK7aek+IMINHJksB73jzt
         RB5FkVSg0TLUV/9KzIveZp+1bbumtH5wjDUFY=
MIME-Version: 1.0
In-Reply-To: <1259669142.2291.9.camel@guybrush>
References: <b65d742b0911300532g1b60034cn30b56c10d88e7cde@mail.gmail.com>
	 <1259669142.2291.9.camel@guybrush>
Date: Thu, 3 Dec 2009 18:00:22 +0530
Message-ID: <b65d742b0912030430md3e582awba44d45b049982b@mail.gmail.com>
To: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [PATCH] - Improving multi-threaded performance by 
	propagating TSRMLS_C
From: yoarvi@gmail.com (Arvind Srinivasan)

>
>> (2) If the number of occurrences is large, then rename the function
>> (prefix with _) and add a macro that passes TSRMLS_C
>> e.g.
>> -ZEND_API void zend_hash_destroy(HashTable *ht);
>> +ZEND_API void _zend_hash_destroy(HashTable *ht TSRMLS_DC);
>> +#define zend_hash_destroy(ht) _zend_hash_destroy((ht) TSRMLS_CC)
>

With the exception of the APIs mentioned below, I've undone this
change and instead done a global search/replace of all the functions
that needed to pass TSRMLS_C as a parameter.

The only cases that I couldn't quite do this for are:
  convert_to_long
  convert_to_double
  convert_to_null
  convert_to_boolean
  convert_to_object
  convert_to_array
in zend_operators.h

The problem is that currently convert_to_string is already a macro
that calls _convert_to_string passing 2 parameters whereas the above
are functions that take only 1 parameter. conv_object_to_type expects
all these APIs to have the same signature.


>> (4) For single argument functions that have varags, pass TSRMLS_C as
>> the first parameter
>> e.g.
>> -ZEND_API int zend_get_parameters_ex(int param_count, ...) /* {{{ */
>> +ZEND_API int zend_get_parameters_ex(TSRMLS_DC1 int param_count, ...) /*=
 {{{ */
>>
>> where
>> +#define TSRMLS_DC1 =A0 TSRMLS_D,
>> +#define TSRMLS_CC1 =A0 TSRMLS_C,
>
> Are there more cases of this? - If zend_get_parameters is the only case
> I'd keep the TSRMLS_FETCH in it. zend_get_parameters is deprecated and
> shouldn't be used anymore. But adding a new macro makes it harder for
> new people to get started with the code.

I've backed out this change as well and instead used TSRMLS_FETCH() in
those APIs that used TSRMLS_DC1/TSRMLS_CC1 (in my previous diff). I'll
revisit those if they show up as performance hotspots.

http://bitbucket.org/arvi/arviq/src/37f1d6b045b0/svn-TSRM-patch.txt
contains the updated diff against trunk.

I've compiled and run 'make test' on this after configuring it with
the default module set and with/without --enable-maintainer-zts.

Thanks,
Arvi