Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: update PHP 8.1, 8.2, and 8.3 to latest patch release #2752

Merged
merged 6 commits into from
Jul 12, 2024

Conversation

realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Jul 9, 2024

Description

This PR will update PHP 8.1, 8.2, and 8.3 to their latest patch releases. This is mostly a cleanup before starting work on PHP 8.4

PROF-10137

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions bot added profiling Relates to the Continuous Profiler tracing area:asm labels Jul 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.95%. Comparing base (2dc6bac) to head (9401ece).
Report is 9 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2752      +/-   ##
============================================
- Coverage     79.51%   77.95%   -1.56%     
  Complexity     2216     2216              
============================================
  Files           201      227      +26     
  Lines         22612    26638    +4026     
  Branches          0      989     +989     
============================================
+ Hits          17980    20766    +2786     
- Misses         4632     5346     +714     
- Partials          0      526     +526     
Flag Coverage Δ
appsec-extension 69.19% <ø> (?)
tracer-extension 78.81% <100.00%> (+<0.01%) ⬆️
tracer-php 80.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ext/hook/uhook.c 84.61% <100.00%> (+0.02%) ⬆️

... and 26 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dc6bac...9401ece. Read the comment docs.

@realFlowControl realFlowControl marked this pull request as ready for review July 9, 2024 15:56
@realFlowControl realFlowControl requested a review from a team as a code owner July 9, 2024 15:56
@morrisonlevi morrisonlevi changed the title ci: update PHP 8.0, 8.1 and 8.2 to latest patch release ci: update PHP 8.1, 8.2, and 8.3 to latest patch release Jul 9, 2024
@morrisonlevi
Copy link
Collaborator

This PHP 8.2 language test failure is concerning:

========DIFF========
001+ Killed
002+ 
001- File written successfully.
002- File read successfully.
003+ Termsig=9
========DONE========
FAIL Test file_put_contents() and file_get_contents() functions with 5GB string [ext/standard/tests/file/file_get_contents_file_put_contents_5gb.phpt] 

Looks like it was added in PHP 8.2.18 or so: php/php-src@2343791.

@morrisonlevi
Copy link
Collaborator

Observer failures are concerning too:

========DIFF========
     foo() suppressed
002+ php: /usr/local/src/php/ext/zend_test/observer.c:73: assert_observer_opline: Assertion `!((((execute_data)->func)->type) != 1) || (((execute_data)->opline) >= ((execute_data)->func)->op_array.opcodes && ((execute_data)->opline) < ((execute_data)->func)->op_array.opcodes + ((execute_data)->func)->op_array.last) || (((execute_data)->opline) >= (((zend_executor_globals *) (((char*) _tsrm_ls_cache)+(executor_globals_offset)))->exception_op) && ((execute_data)->opline) < (((zend_executor_globals *) (((char*) _tsrm_ls_cache)+(executor_globals_offset)))->exception_op) + 3)' failed.
003+ Aborted (core dumped)
004+ 
005+ Termsig=6
002- NULL
003- foo() not suppressed
004- string(19) "function was called"
005- fooStr() suppressed
006- string(15) "overriden value"
007- fooStr() not suppressed
008- fooStr() function was called
009- string(19) "function was called"
========DONE========
FAIL Suppress function call via suppressCall() [tmp/build_extension/tests/ext/sandbox/install_hook/suppress_call.phpt] 
PASS Gracefully handle out-of-sync spans from traced function [internal][default properties] [tmp/build_extension/tests/ext/sandbox/spans_out_of_sync_02.phpt] 
PASS Gracefully handle out-of-sync spans from traced function [user] [tmp/build_extension/tests/ext/sandbox/spans_out_of_sync_03.phpt] 
                                                   
========DIFF========
     With hook
002+ php: /usr/local/src/php/ext/zend_test/observer.c:73: assert_observer_opline: Assertion `!((((execute_data)->func)->type) != 1) || (((execute_data)->opline) >= ((execute_data)->func)->op_array.opcodes && ((execute_data)->opline) < ((execute_data)->func)->op_array.opcodes + ((execute_data)->func)->op_array.last) || (((execute_data)->opline) >= (((zend_executor_globals *) (((char*) _tsrm_ls_cache)+(executor_globals_offset)))->exception_op) && ((execute_data)->opline) < (((zend_executor_globals *) (((char*) _tsrm_ls_cache)+(executor_globals_offset)))->exception_op) + 3)' failed.
003+ Aborted (core dumped)
002- Without hook
003- string(6) "called"
004+ 
005+ Termsig=6
========DONE========
FAIL suppressCall() works with JIT [tmp/build_extension/tests/ext/sandbox/install_hook/suppress_call_jit.phpt] 

@realFlowControl
Copy link
Member Author

This PHP 8.2 language test failure is concerning:

========DIFF========
001+ Killed
002+ 
001- File written successfully.
002- File read successfully.
003+ Termsig=9
========DONE========
FAIL Test file_put_contents() and file_get_contents() functions with 5GB string [ext/standard/tests/file/file_get_contents_file_put_contents_5gb.phpt] 

Looks like it was added in PHP 8.2.18 or so: php/php-src@2343791.

We get OOM killed 😉
The current limit is 4 GB on medium instances.

php/php-src#14895

I rerun the job and this test was not executed due to race conditions with checking /proc/meminfo with most likely other containers on that same host.

I'll add that test to our skip list

@realFlowControl
Copy link
Member Author

The tmp/build_extension/tests/ext/sandbox/install_hook/suppress_call.phpt and tmp/build_extension/tests/ext/sandbox/install_hook/suppress_call_jit.phpt seems stable and gdb gives me:

foo() suppressed
php: /usr/local/src/php/ext/zend_test/observer.c:73: assert_observer_opline: Assertion `!((((execute_data)->func)->type) != 1) || (((execute_data)->opline) >= ((execute_data)->func)->op_array.opcodes && ((execute_data)->opline) < ((execute_data)->func)->op_array.opcodes + ((execute_data)->func)->op_array.last) || (((execute_data)->opline) >= (((zend_executor_globals *) (((char*) _tsrm_ls_cache)+(executor_globals_offset)))->exception_op) && ((execute_data)->opline) < (((zend_executor_globals *) (((char*) _tsrm_ls_cache)+(executor_globals_offset)))->exception_op) + 3)' failed.

Thread 1 "php" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff433c535 in __GI_abort () at abort.c:79
#2  0x00007ffff433c40f in __assert_fail_base (fmt=0x7ffff449def0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=0x7ffff1159c40 "!((((execute_data)->func)->type) != 1) || (((execute_data)->opline) >= ((execute_data)->func)->op_array.opcodes && ((execute_data)->opline) < ((execute_data)->func)->op_array.opcodes + ((execute_data)"..., file=0x7ffff1159be0 "/usr/local/src/php/ext/zend_test/observer.c", line=73, function=<optimized out>) at assert.c:92
#3  0x00007ffff434a1a2 in __GI___assert_fail (
    assertion=0x7ffff1159c40 "!((((execute_data)->func)->type) != 1) || (((execute_data)->opline) >= ((execute_data)->func)->op_array.opcodes && ((execute_data)->opline) < ((execute_data)->func)->op_array.opcodes + ((execute_data)"..., file=0x7ffff1159be0 "/usr/local/src/php/ext/zend_test/observer.c", line=73, function=0x7ffff115ad40 <__PRETTY_FUNCTION__.21009> "assert_observer_opline") at assert.c:101
#4  0x00007ffff1148edd in assert_observer_opline (execute_data=0x7ffff00a6a10) at /usr/local/src/php/ext/zend_test/observer.c:71
#5  0x00007ffff1149c89 in observer_end (execute_data=0x7ffff00a6a10, retval=0x7ffff00a68b0) at /usr/local/src/php/ext/zend_test/observer.c:123
#6  0x0000555556f82380 in call_end_observers (execute_data=0x7ffff00a6a10, return_value=0x7ffff00a68b0) at /usr/local/src/php/Zend/zend_observer.c:274
#7  0x0000555556f823f3 in zend_observer_fcall_end (execute_data=0x7ffff00a6a10, return_value=0x7ffff00a68b0) at /usr/local/src/php/Zend/zend_observer.c:283
#8  0x0000555556ec7fc2 in execute_ex (ex=0x7ffff00a6820) at /usr/local/src/php/Zend/zend_vm_execute.h:56437
#9  0x0000555556ed83f2 in zend_execute (op_array=0x61100001d000, return_value=0x0) at /usr/local/src/php/Zend/zend_vm_execute.h:60439
#10 0x0000555556cc536b in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /usr/local/src/php/Zend/zend.c:1840
#11 0x0000555556b06cfa in php_execute_script (primary_file=0x7fffffffbbf0) at /usr/local/src/php/main/main.c:2565
#12 0x00005555570b657b in do_cli (argc=86, argv=0x617000000080) at /usr/local/src/php/sapi/cli/php_cli.c:964
#13 0x00005555570b8795 in main (argc=86, argv=0x617000000080) at /usr/local/src/php/sapi/cli/php_cli.c:1333

@bwoebi do you have an idea? I have a gdb session open currently and am tinkering with it

@pr-commenter
Copy link

pr-commenter bot commented Jul 10, 2024

Benchmarks

Benchmark execution time: 2024-07-11 16:06:41

Comparing candidate commit 9401ece in PR branch florian/ci-container-update with baseline commit 2dc6bac in branch master.

Found 5 performance improvements and 0 performance regressions! Performance is the same for 173 metrics, 0 unstable metrics.

scenario:BM_TeaSapiSpindown

  • 🟩 execution_time [-32.242µs; -13.359µs] or [-6.049%; -2.506%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-8.114µs; -5.166µs] or [-3.292%; -2.096%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟩 execution_time [-7.934µs; -5.766µs] or [-3.267%; -2.375%]

scenario:PDOBench/benchPDOBaseline

  • 🟩 execution_time [-17.605µs; -12.532µs] or [-9.610%; -6.841%]

scenario:PDOBench/benchPDOOverhead

  • 🟩 execution_time [-17.488µs; -11.533µs] or [-6.144%; -4.052%]

@realFlowControl realFlowControl force-pushed the florian/ci-container-update branch 11 times, most recently from 2c669d8 to 680a38c Compare July 11, 2024 09:23
@realFlowControl realFlowControl force-pushed the florian/ci-container-update branch 16 times, most recently from e46b47e to 89e6ab6 Compare July 11, 2024 13:18
.gitlab/ci-images.yml Outdated Show resolved Hide resolved
@realFlowControl realFlowControl force-pushed the florian/ci-container-update branch from 89e6ab6 to eb383d7 Compare July 11, 2024 13:44
@realFlowControl
Copy link
Member Author

@bwoebi / @morrisonlevi could you have another look at the PR?
I removed building Windows containers in CI as long as there is no docker compose support, will re-add this once there is support for docker compose.
@bwoebi can you build the images?

@realFlowControl realFlowControl merged commit 4c3832d into master Jul 12, 2024
665 of 668 checks passed
@realFlowControl realFlowControl deleted the florian/ci-container-update branch July 12, 2024 10:08
@github-actions github-actions bot added this to the 1.2.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci profiling Relates to the Continuous Profiler tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants