Hi Anatol,

Thank you for update.
I'm surprised you don't see speed difference even with ZTS :(

I didn't get what you propose to change in virtual_file_ex().

I'll do a more careful patch review later on this week.

Thaks. Dmitry.


On Mon, Oct 28, 2013 at 10:31 PM, Anatol Belski <a...@php.net> wrote:

> Hi Dmitry,
>
> On Wed, October 23, 2013 14:55, Dmitry Stogov wrote:
> > Hi Anatol,
> >
> >
> > First of all PHP is mainly used as NTS on Linux (LAMP stack)
> > It's the reason I tested it first, to check if your patch affects the
> > performance that is really important for users.
> >
> > I still think that tsrm_do_alloca() have to be replaced by do_alloca().
> > They have exactly the same logic, except that the first one fallback to
> > malloc() and the second one to emalloc(). It must not be affected by
> > malloc()s and realloc()s if you changed them into emalloc() and
> > erealloc(). May be I missing something...
> >
> >
> > I'm not sure about alloca() usage on Windows, but you may change zend.h
> > if you like. BTW: I don't think we should replace alloca() by _malloca(),
> > because do_alloca() already does the same in portable way and uses
> > per-request heap.
> >
>
> last week I've updated the patch according to your recommendations, now we
> have the performance test results
>
>
> http://windows.php.net/downloads/snaps/ostc/pftt/perf/results-20131025-MasterVanilla-Master50333-3820.html
>
> http://windows.php.net/downloads/snaps/ostc/pftt/perf/results-20131025-MasterVanilla-Master50333-7795.html
>
> Also enabled the stack usage in both TS/NTS for windows. It shows still no
> improvement as one can see. Though, there are some functions, like
> virtual_file_ex(), which use emalloc() only in the patch, but called very
> often within the virtual cwd API. Changing them would require signature
> changes os one can efree() in case of the fallback situation, but still
> it's doubtful that would tell some bigger difference in performance. Do
> you think it's worth a try?
>
> Regards
>
> Anatol
>
>

Reply via email to