Re: Fix for bug #50333

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 >