Since we are on the topic of reviewing past RFCs for 5.4, can we take
another look at the Zend Signals RFC:
https://wiki.php.net/rfc/zendsignals
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?
Hi!
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?
I don't know of any. Are there any issues with this change (BC, etc.)?
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
I do not believe so.
Hi!
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?I don't know of any. Are there any issues with this change (BC, etc.)?
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
I do not believe so.
Then I guess if nobody has any objections we can do it.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?I don't know of any. Are there any issues with this change (BC, etc.)?
There could be some weird interactivity issues with certain
environments, but the patch takes care of most issues and I think the
added stability this gives opcode caches is worth the minor risk.
-Rasmus
hi Ilia,
I would suggest to kill the TSRMLS_FETCH while being at it. They are
horribly slow and a couple of them can be replaced by the
TSRMLS_DC/CC, if I'm not mistaken.
For the windows side, I do not have the time to do the equivalent, so
if you commit the patch to trunk first so I can fix the build
accordingly and then merge, that would be easier for me :).
Cheers,
Since we are on the topic of reviewing past RFCs for 5.4, can we take
another look at the Zend Signals RFC:https://wiki.php.net/rfc/zendsignals
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?--
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.
hi Ilia,
I would suggest to kill the TSRMLS_FETCH while being at it. They are
horribly slow and a couple of them can be replaced by the
TSRMLS_DC/CC, if I'm not mistaken.For the windows side, I do not have the time to do the equivalent, so
if you commit the patch to trunk first so I can fix the build
accordingly and then merge, that would be easier for me :).Cheers,
Since we are on the topic of reviewing past RFCs for 5.4, can we take
another look at the Zend Signals RFC:https://wiki.php.net/rfc/zendsignals
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?--
--
Pierre@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-)
I mean in this patch only. This patch adds a couple, so it can be done
at the same time (afair these functions are not used heavily outside
the engine).
And for the record I am all for killing
TSRMLS_FETCH.
Same here (tls and some other ideas may help :)
hi Ilia,
I would suggest to kill the TSRMLS_FETCH while being at it. They are
horribly slow and a couple of them can be replaced by the
TSRMLS_DC/CC, if I'm not mistaken.For the windows side, I do not have the time to do the equivalent, so
if you commit the patch to trunk first so I can fix the build
accordingly and then merge, that would be easier for me :).Cheers,
Since we are on the topic of reviewing past RFCs for 5.4, can we take
another look at the Zend Signals RFC:https://wiki.php.net/rfc/zendsignals
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?--
--
Pierre@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky ilia@prohost.org
escreveu:
Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.
Is there any advantage in killing it as opposed to simply not use it?
You will be breaking a lot of extensions. Some of them depend on libraries
with functions that receive callbacks to which you cannot pass specific
user data, which unnecessarily complicates refactoring.
--
Gustavo Lopes
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky ilia@prohost.org
escreveu:Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.Is there any advantage in killing it as opposed to simply not use it?
I think he meant just replacing it in this patch.
--
Cheers,
Michael
Hi,
2011/6/2 Michael Maclean michael@no-surprises.co.uk
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky ilia@prohost.org
escreveu:Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.Is there any advantage in killing it as opposed to simply not use it?
I think he meant just replacing it in this patch.
Just to inform, with the patched applied in trunk we have 4 SIGSEGVs with
ext/pcntl tests:
pcntl_alarm() [ext/pcntl/tests/pcntl_alarm.phpt]
pcntl_signal() [ext/pcntl/tests/pcntl_signal.phpt]
pcnt_signal_dispatch() [ext/pcntl/tests/pcntl_signal_dispatch.phpt]
Closures as a signal handler [ext/pcntl/tests/signal_closure_handler.phpt]
And 1 test hanging:
ext/pcntl/tests/002.phpt
--
Regards,
Felipe Pena
2011/6/2 Felipe Pena felipensp@gmail.com
Hi,
2011/6/2 Michael Maclean michael@no-surprises.co.uk
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky ilia@prohost.org
escreveu:Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.Is there any advantage in killing it as opposed to simply not use it?
I think he meant just replacing it in this patch.
Just to inform, with the patched applied in trunk we have 4 SIGSEGVs with
ext/pcntl tests:
pcntl_alarm()[ext/pcntl/tests/pcntl_alarm.phpt]
pcntl_signal()[ext/pcntl/tests/pcntl_signal.phpt]
pcnt_signal_dispatch() [ext/pcntl/tests/pcntl_signal_dispatch.phpt]
Closures as a signal handler [ext/pcntl/tests/signal_closure_handler.phpt]And 1 test hanging:
ext/pcntl/tests/002.phpt
Ok, already fixed. There is only a test failing due a behavior change:
$ cat ext/pcntl/tests/pcntl_signal.diff
009+ Fatal error: Error installing signal handler for -1 in
/home/felipe/dev/phptrunk/ext/pcntl/tests/pcntl_signal.php on line 10
009- Warning: pcntl_signal(): Error assigning signal %s
010- bool(false)
011-
012- Warning: pcntl_signal(): Error assigning signal %s
013- bool(false)
014-
015- Warning: pcntl_signal(): not callable is not a callable function name
error in %s
016- bool(false)
017- ok
--
Regards,
Felipe Pena
The crash is now fixed as well.
2011/6/2 Felipe Pena felipensp@gmail.com
Hi,
2011/6/2 Michael Maclean michael@no-surprises.co.uk
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky ilia@prohost.org
escreveu:Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.Is there any advantage in killing it as opposed to simply not use it?
I think he meant just replacing it in this patch.
Just to inform, with the patched applied in trunk we have 4 SIGSEGVs with
ext/pcntl tests:
pcntl_alarm()[ext/pcntl/tests/pcntl_alarm.phpt]
pcntl_signal()[ext/pcntl/tests/pcntl_signal.phpt]
pcnt_signal_dispatch() [ext/pcntl/tests/pcntl_signal_dispatch.phpt]
Closures as a signal handler [ext/pcntl/tests/signal_closure_handler.phpt]And 1 test hanging:
ext/pcntl/tests/002.phptOk, already fixed. There is only a test failing due a behavior change:
$ cat ext/pcntl/tests/pcntl_signal.diff
009+ Fatal error: Error installing signal handler for -1 in
/home/felipe/dev/phptrunk/ext/pcntl/tests/pcntl_signal.php on line 10
009- Warning:pcntl_signal(): Error assigning signal %s
010- bool(false)
011-
012- Warning:pcntl_signal(): Error assigning signal %s
013- bool(false)
014-
015- Warning:pcntl_signal(): not callable is not a callable function name
error in %s
016- bool(false)
017- ok--
Regards,
Felipe Pena
Fixed invalid sigaction() call passing NSIG as signal number.
- for (signo = 1; signo <= NSIG; ++signo) {
- for (signo = 1; signo < NSIG; ++signo) {
Detected by Valgrind:
==4577== Warning: bad signal number 65 in sigaction()
2011/6/3 Ilia Alshanetsky ilia@prohost.org
The crash is now fixed as well.
2011/6/2 Felipe Pena felipensp@gmail.com
Hi,
2011/6/2 Michael Maclean michael@no-surprises.co.uk
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky <
ilia@prohost.org>
escreveu:Killing TSRMLS_FETCH is a noble goal, but let's keep it to once patch
at a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.Is there any advantage in killing it as opposed to simply not use it?
I think he meant just replacing it in this patch.
Just to inform, with the patched applied in trunk we have 4 SIGSEGVs
with
ext/pcntl tests:
pcntl_alarm()[ext/pcntl/tests/pcntl_alarm.phpt]
pcntl_signal()[ext/pcntl/tests/pcntl_signal.phpt]
pcnt_signal_dispatch() [ext/pcntl/tests/pcntl_signal_dispatch.phpt]
Closures as a signal handler
[ext/pcntl/tests/signal_closure_handler.phpt]And 1 test hanging:
ext/pcntl/tests/002.phptOk, already fixed. There is only a test failing due a behavior change:
$ cat ext/pcntl/tests/pcntl_signal.diff
009+ Fatal error: Error installing signal handler for -1 in
/home/felipe/dev/phptrunk/ext/pcntl/tests/pcntl_signal.php on line 10
009- Warning:pcntl_signal(): Error assigning signal %s
010- bool(false)
011-
012- Warning:pcntl_signal(): Error assigning signal %s
013- bool(false)
014-
015- Warning:pcntl_signal(): not callable is not a callable function
name
error in %s
016- bool(false)
017- ok--
Regards,
Felipe Pena
--
Regards,
Felipe Pena
Fixed crash in fastcgi due startup order...
SIGG() were being used before tsrm_startup().
2011/6/4 Felipe Pena felipensp@gmail.com
Fixed invalid sigaction() call passing NSIG as signal number.
- for (signo = 1; signo <= NSIG; ++signo) {
- for (signo = 1; signo < NSIG; ++signo) {
Detected by Valgrind:
==4577== Warning: bad signal number 65 in sigaction()2011/6/3 Ilia Alshanetsky ilia@prohost.org
The crash is now fixed as well.
2011/6/2 Felipe Pena felipensp@gmail.com
Hi,
2011/6/2 Michael Maclean michael@no-surprises.co.uk
Em Thu, 02 Jun 2011 18:10:50 +0100, Ilia Alshanetsky <
ilia@prohost.org>
escreveu:Killing TSRMLS_FETCH is a noble goal, but let's keep it to once
patchat a time please ;-) And for the record I am all for killing
TSRMLS_FETCH.Is there any advantage in killing it as opposed to simply not use it?
I think he meant just replacing it in this patch.
Just to inform, with the patched applied in trunk we have 4 SIGSEGVs
with
ext/pcntl tests:
pcntl_alarm()[ext/pcntl/tests/pcntl_alarm.phpt]
pcntl_signal()[ext/pcntl/tests/pcntl_signal.phpt]
pcnt_signal_dispatch() [ext/pcntl/tests/pcntl_signal_dispatch.phpt]
Closures as a signal handler
[ext/pcntl/tests/signal_closure_handler.phpt]And 1 test hanging:
ext/pcntl/tests/002.phptOk, already fixed. There is only a test failing due a behavior change:
$ cat ext/pcntl/tests/pcntl_signal.diff
009+ Fatal error: Error installing signal handler for -1 in
/home/felipe/dev/phptrunk/ext/pcntl/tests/pcntl_signal.php on line 10
009- Warning:pcntl_signal(): Error assigning signal %s
010- bool(false)
011-
012- Warning:pcntl_signal(): Error assigning signal %s
013- bool(false)
014-
015- Warning:pcntl_signal(): not callable is not a callable function
name
error in %s
016- bool(false)
017- ok--
Regards,
Felipe Pena--
Regards,
Felipe Pena
--
Regards,
Felipe Pena
Since we are on the topic of reviewing past RFCs for 5.4, can we take
another look at the Zend Signals RFC:https://wiki.php.net/rfc/zendsignals
The patch is solid (have been using it in production for quite some
time) and improvement is quite helpful, especially when APC is being
used. Are there any reasons not to apply this to 5.4?
Can you post an updated patch?
Chris
--
Email: christopher.jones@oracle.com
Tel: +1 650 506 8630
Blog: http://blogs.oracle.com/opal/