Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:46249
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.222.176 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:date:message-id:subject:from:to:content-type;
        b=jSb3XZ9qWWhmq21Cq/Shmw71/F392JJpgIsssU1EU8nTmfJl6YfbChedsieeTN2guj
         9zkowRLd/aFuwhxJDfLvKCmsDV3wJVsY65quHCRBCjiJvAZ/7pNIFx6BYjen/6FmwicR
         Ivm6uwoTFT3b6xH/8uctMAEw9+T0yJo7fcRo0=
MIME-Version: 1.0
Date: Mon, 30 Nov 2009 19:02:27 +0530
Message-ID: <b65d742b0911300532g1b60034cn30b56c10d88e7cde@mail.gmail.com>
To: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Subject: [PATCH] - Improving multi-threaded performance by propagating 
	TSRMLS_C
From: yoarvi@gmail.com (Arvind Srinivasan)

When running a benchmarking workload on PHP that was configured with
multi-threading support (--enable-maintainer-zts) I noticed that
pthread_get_specific is invoked many times during the processing of a
request. PHP code invokes TSRMLS_FETCH() (which ends up invoking
ts_resource_ex) in a number of places.

Caller/callee data from Sun Studio's collector/analyzer showed the following:

Attr.     Excl.     Incl.      Name
User CPU  User CPU  User CPU
   sec.      sec.       sec.
178.105   185.460    363.564   _emalloc
 96.568   114.320    210.888   _efree
 27.960    89.232    343.901   _zval_ptr_dtor
 19.544     6.685     26.228   php_body_write_wrapper
  6.925    36.806    224.617   _zval_dtor_func
  4.263     2.902      7.165   safe_free_zval_ptr_rel
  4.163    11.898     16.061   zend_get_parameters_ex
  4.013    14.690    174.682   my_copy_zval
  3.963     6.775     10.738   _erealloc
  3.502    12.399    978.444   apc_copy_function_for_execution
  2.732     4.143      9.647   do_inherit_method_check
  2.592    21.565    225.137   _zval_copy_ctor_func
  0.881    22.095    106.535   virtual_file_ex
  0.600     1.961      6.855   list_entry_destructor
  0.470     1.301     24.397   zend_file_handle_dtor
  0.410     1.781      2.712   zend_function_dtor
  0.270     0.350      0.620   convert_to_array
  0.220     0.991     15.831   apc_search_paths
  0.150     0.490      3.362   zend_register_resource
  0.140     1.581     10.137   zend_alter_ini_entry
  0.130     4.833   9023.272   php5_execute
  0.110     0.500      3.502   zend_ini_long
  0.070     0.530      0.600   _zend_bailout
  0.050     0.320      4.513   zend_error
  0.040     0.690      3.913   php_error_cb
  0.040     0.560      2.852   zend_alter_ini_entry_ex
  0.        3.202    584.369   php_request_shutdown
274.252   274.252    357.910  *ts_resource_ex
 83.659    84.749     84.749   pthread_getspecific

Propagating the value of TSRMLS_CC will avoid the overhead of having
to invoke tsrm_tls_get/pthread_get_specific. The following patch
(against trunk) does this:
http://bitbucket.org/arvi/arviq/src/tip/svn-TSRM-patch.txt

The above patch is mostly the result of making zend_alloc.[ch] TSRMLS_C-aware.

While modifying the APIs to add propagate TSRMLS_C, I didn't quite
know the best/preferred way to make the changes so I tried to use the
following guidelines:

(1) If the number of occurrences of the API to be modified are
relatively small, then make the change directly.
e.g.
-int fcgi_accept_request(fcgi_request *req);
-int fcgi_finish_request(fcgi_request *req, int force_close);
+int fcgi_accept_request(fcgi_request *req TSRMLS_DC);
+int fcgi_finish_request(fcgi_request *req, int force_close TSRMLS_DC);

(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)

(3) For functions that have varargs, pass TSRMLS_C as the last but one parameter
e.g.
-ZEND_API int zend_get_parameters(int ht, int param_count, ...);
+ZEND_API int zend_get_parameters(int ht TSRMLS_DC, int param_count, ...);

(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	TSRMLS_D,
+#define TSRMLS_CC1	TSRMLS_C,

Is (2) an acceptable thing to do or should I avoid renaming functions
and do a mongo search-replace?

With respect to (4), I wasn't sure how else to tackle varargs
functions that take a single parameter. Any direction on what I should
do for (3), (4) would be much appreciated.

Out of curiosity, I counted the number of times
		thread_resources = tsrm_tls_get();
in ts_resource_ex is invoked for a simple command line invocation of
hello.php (<?php echo "Hello World"?>).

Before patch: 22125 times
After patch: 119 times

Most of this is probably in one-time initialization stuff. I hope to
have my trunk tree hooked up to my benchmarking setup soon and then
I'll have more meaningful numbers.

Thanks,
Arvi