#70497: Drop Apache 1 style BLOCK_INTERRUPTIONS support #1520
Conversation
This does the same thing as 127db5a ("HANDLE_BLOCK_INTERRUPTIONS() is not used by SAPIs anymore. It may be useful only when PHP configured with --enable-zend-signals.") In the PHP7 version, the code could be dropped easily because no SAPI used the hooks. However, in PHP5, the Apache 1 SAPIs use these hooks. Some other SAPIs provided NULL pointers. The Apache 1 SAPIs now throw a warning on build in case anyone still uses them. The other SAPIs have had the relevant SAPI struct entries dropped and so should still build. This leads to a speed up on PHP5 code. I've measured the performance impact on a PowerPC 64-bit little-endian system and on a x86_64 laptop using the following script: <?php for ($i=1; $i < 100000000; $i++) { $genericObject = new stdClass(); $a = (object) 8; $b = (object) 'abcde'; } ?> I see a ~6% improvement in performance on POWER and a ~4% improvement on x86_64. I also see an improvement in a e-commerce web application on POWER with this change. Signed-off-by: Daniel Axtens <dja@axtens.net>
|
This was already dropped for PHP 7 (the actual calls at least). I don't think we should be doing a change like this in 5.6. |
|
Yes, the same change was made in PHP 7: this essentially backports it to 5.6. I think it is worthwhile because it is a noticeable performance improvement. Why do you think it shouldn't go in? |
|
This affects Apache 1 SAPI. It's dropped in PHP-7 but not in PHP-5.6. On Thu, Sep 17, 2015 at 1:28 AM, Daniel Axtens notifications@github.com
|
|
I know Apache 1 is still supported, but I don't think any distros have shipped Apache 1 for a very long time. (Apache 2 was released in 2002, and Apache 1 was EOLed in 2010.) Also, the Apache 1 SAPI will still build and run with this change. The only potential issue this would cause would be the chance of memory leaks/etc that are guarded by HANDLE_BLOCK_INTERRUPTIONS(). If you want, I could do a version 2 of this pull request that checks if Apache 1 support is enabled via #ifdef, and preserves the original behaviour in that case. I know this is dropped in PHP 7 and PHP 7 performance is already much better. However, lots of people will still be using 5.6 for a long time, so it makes sense for it to have good performance. This patch makes a measurable difference for a micro-benchmark and for a real workload. |
|
I would prefer not to make such changes in minor 5.6.* releases. On Fri, Sep 18, 2015 at 1:33 AM, Daniel Axtens notifications@github.com
|
|
OK, I understand the desire not to make behavioural changes. I can do a version 2 that will make no change to behaviour:
This way, we can be sure that the behaviour will be identical in all cases. |
|
Version 2 is at #1532 |
This does the same thing as 127db5a ("HANDLE_BLOCK_INTERRUPTIONS()
is not used by SAPIs anymore. It may be useful only when PHP configured
with --enable-zend-signals.")
In the PHP7 version, the code could be dropped easily because no SAPI
used the hooks. However, in PHP5, the Apache 1 SAPIs use these hooks.
Some other SAPIs provided NULL pointers.
The Apache 1 SAPIs now throw a warning on build in case anyone still
uses them. The other SAPIs have had the relevant SAPI struct entries
dropped and so should still build.
This leads to a speed up on PHP5 code. I've measured the performance
impact on a PowerPC 64-bit little-endian system and on a x86_64 laptop
using the following script:
I see a ~6% improvement in performance on POWER and a ~4% improvement
on x86_64.
I also see an improvement in a e-commerce web application on POWER
with this change.
Signed-off-by: Daniel Axtens dja@axtens.net