Hi Lucas,
I took a look into patch and I still don't like it.
I may miss some things and make mistakes so correct me if I'm wrong.
1) It makes some slowdown for all SAPIs except Apache1, because it adds
additional block/unblock code (mainly in zend_alloc.c)
2) It makes ~10 additional syscalls on each request. I didn't get why do
you save original handlers in MINIT and set new handlers in RINIT. In
general they can be changed between MINIT and RINIT by other apache
module or web server so they should be saved/set in RINIT, but from
performance point of view it might be better to save/set signals in MINIT.
3) The patch cannot solve all issues caused by SIGALARM handling,
because PHP has thousand places which may stay data in inconsistent
state (or data allocated in permanent heap). Wrapping all of them with
block/unblock is not a decision.
small issues:
4) block/unblock code in estrndup() and some other functions is useless.
It should be removed.
5) TSRM_FETCH() in zend_hash and zend_alloc functions will slowdown
Windows ZTS build even if it doesn't use signals. It may be changed to
some other macro that will do TSRM_FETCH only if signals are enabled.
May be if we really like safe SIGALARM handling we may use Windows
decision for UNIX too. I mean setup of EG(timed_out) flag in signal
handler and checking it in execute loop. As an optimization we can check
it not on each instruction but only on enter in function and JMP*
opcodes (loops). Anyway, this solution may cause comparable slowdown.
Thanks. Dmitry.
Lucas Nealan wrote:
On 8/11/08 8:52 AM, "Dmitry Stogov" <[EMAIL PROTECTED]> wrote:
I'll try to review it on Tuesday/Wednesday.
Thanks. Dmitry.
I've just updated the patches. Only some very minor changes as discussed
before and they should cleanly apply against current cvs.
-lucas
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
