Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:69730
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain zend.com from 209.85.212.177 cause and error)
MIME-Version: 1.0
In-Reply-To: <c35dcb527aee65fdc17f1a29aadd8d21.squirrel@webmail.klapt.com>
References: <c35dcb527aee65fdc17f1a29aadd8d21.squirrel@webmail.klapt.com>
Date: Mon, 21 Oct 2013 16:30:28 +0400
Message-ID: <CA+9eiLskBJGsMuW_-Pc1Scf3s7edxvEAeoz-kjj95Ui7xZu89g@mail.gmail.com>
To: Anatol Belski <ab@php.net>
Cc: Pierre Joye <pierre.php@gmail.com>, PHP Internals <internals@lists.php.net>, 
	Andi Gutmans <andi@zend.com>, Zeev Suraski <zeev@zend.com>, Xinchen Hui <xinchen.h@zend.com>
Content-Type: multipart/alternative; boundary=047d7bb04986089d1004e93f7327
Subject: Re: Fix for bug #50333
From: dmitry@zend.com (Dmitry Stogov)

--047d7bb04986089d1004e93f7327
Content-Type: text/plain; charset=UTF-8

Hi,

I don't have strong opinion about the patch.
I thought malloc() -> emalloc() change might improve PHP performance in
general, but unfortunately it doesn't.
On the other hand the patch is quite big and introduces source level
incompatibility.

The patch is not complete. At least it misses this chunk:

--- a/sapi/cgi/cgi_main.c
+++ b/sapi/cgi/cgi_main.c
@@ -1396,7 +1396,7 @@ static void init_request_info(fcgi_request *request
TSRMLS_DC)
                                } else {
                                        SG(request_info).request_uri =
env_script_name;
                                }
-                               free(real_path);
+                               efree(real_path);
                        }
                } else {
                        /* pre 4.3 behaviour, shouldn't be used but
provides BC */

It's also much better to use do_alloca() instead of tsrm_do_alloca() (the
patch changed them to less efficient emalloc()).
May be if you change it, we would see improvement :)

As I said, currently, the patch doesn't significantly affect performance of
non-thread-safe build on Linux.


master patched improvement  Blog (req/sec) 106.1 105.1 -0.94%  drupal
(req/sec) 1660.5 1668.6 0.49%  fw (req/sec) 231.7 227.499 -1.81%  hello
(req/sec) 11828.8 11980.5 1.28%  qdig (req/sec) 470 477.3 1.55%  typo3
(req/sec) 580.1 579.3 -0.14%  wordpress (req/sec) 185.9 188.5 1.40%  xoops
(req/sec) 130 131.2 0.92%  scrum (req/sec) 185.199 185 -0.11%  ZF1 Hello
(req/sec) 1154.7 1155.2 0.04%  ZF2 Test (req/sec) 248.7 250.7 0.80%
Thanks. Dmitry.



On Fri, Oct 18, 2013 at 7:33 PM, Anatol Belski <ab@php.net> wrote:

> Hi,
>
> the pull request https://github.com/php/php-src/pull/500 fixing the bug
> #50333 is ready to review. Manual tests done so far on linux and windows
> in TS and NTS mode with CLI and Apache show no regression. The performance
> tests are to be done yet.
>
> Regards
>
> Anatol
>

--047d7bb04986089d1004e93f7327--