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.

Sign up for GitHub

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

Jump to bottom

Improve jit tests #12425

Closed
wants to merge 15 commits into from
Closed

Improve jit tests #12425

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a3b0be8
Add phpseclib, Psalm, PHPStan nightly tests
danog Oct 10, 2023
37f4aab
Improve regex
danog Oct 11, 2023
da1bd74
Merge remote-tracking branch 'origin/PHP-8.1' into improve_jit_tests
danog Oct 12, 2023
db8c94e
Update circleCI config
danog Oct 12, 2023
cb1fb30
Fixup
danog Oct 12, 2023
daea65b
Fixup
danog Oct 12, 2023
13e8a3c
Merge remote-tracking branch 'origin/PHP-8.1' into improve_jit_tests
danog Oct 13, 2023
e958183
Improve config
danog Oct 13, 2023
1048945
Fixup
danog Oct 13, 2023
056c63f
Rename & cleanup
danog Oct 19, 2023
745a346
Do not use fixed tag
danog Oct 19, 2023
99d54c9
Revert
danog Oct 22, 2023
c1af7fe
Use same JIT buffer size in all tests
danog Nov 11, 2023
d7c6551
Merge remote-tracking branch 'origin/PHP-8.2' into improve_jit_tests
danog Nov 11, 2023
7c35921
Update
danog Nov 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ jobs:
sapi/cli/php run-tests.php \
-d zend_extension=opcache.so \
-d opcache.enable_cli=1 \
-d opcache.jit_buffer_size=16M \
-d opcache.jit_buffer_size=64M \
danog marked this conversation as resolved.
Show resolved Hide resolved
-d opcache.jit=tracing \
-P -q -x -j2 \
-g FAIL,BORK,LEAK,XLEAK \
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/test-linux/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ runs:
export SKIP_IO_CAPTURE_TESTS=1
sapi/cli/php run-tests.php -P -q ${{ inputs.runTestsParameters }} \
-d opcache.jit=${{ inputs.jitType }} \
-d opcache.jit_buffer_size=16M \
-d opcache.jit_buffer_size=64M \
-j$(/usr/bin/nproc) \
-g FAIL,XFAIL,BORK,WARN,LEAK,XLEAK,SKIP \
--offline \
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/test-macos/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ runs:
export CI_NO_IPV6=1
sapi/cli/php run-tests.php -P -q ${{ inputs.runTestsParameters }} \
-d opcache.jit=${{ inputs.jitType }} \
-d opcache.jit_buffer_size=16M \
-d opcache.jit_buffer_size=64M \
-j$(sysctl -n hw.ncpu) \
-g FAIL,XFAIL,BORK,WARN,LEAK,XLEAK,SKIP \
--offline \
Expand Down
2 changes: 1 addition & 1 deletion .github/scripts/windows/test_task.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ set OPENSSL_CONF=
rem set SSLEAY_CONF=

rem prepare for OPcache
if "%OPCACHE%" equ "1" set OPCACHE_OPTS=-d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.protect_memory=1 -d opcache.jit_buffer_size=16M
if "%OPCACHE%" equ "1" set OPCACHE_OPTS=-d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.protect_memory=1 -d opcache.jit_buffer_size=64M -d opcache.jit=tracing
rem work-around for failing to dl(mysqli) with OPcache (https://github.com/php/php-src/issues/8508)
if "%OPCACHE%" equ "1" set OPCACHE_OPTS=%OPCACHE_OPTS% -d extension=mysqli

Expand Down
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ before_script:

# Run PHPs run-tests.php
script:
- ./travis/test.sh -d opcache.jit_buffer_size=16M -d opcache.jit=tracing
- travis_wait 60 ./travis/test.sh -d opcache.jit_buffer_size=64M -d opcache.jit=tracing
danog marked this conversation as resolved.
Show resolved Hide resolved
- sapi/cli/php -d extension_dir=`pwd`/modules -r 'dl("zend_test");'

after_success:
Expand Down
6 changes: 6 additions & 0 deletions run-tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,12 @@ function main(): void
'opcache.jit_hot_func=1',
'opcache.jit_hot_return=1',
'opcache.jit_hot_side_exit=1',
'opcache.jit_max_root_traces=100000',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need this? Tests are small, it's unlikely we're hitting jit_max_root_traces. Did you verify this for any test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to use the same parameters everywhere, to avoid hitting any limit and not compiling any trace, in case the unit tests were to include some bigger integration tests in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setting may disable some tests in case they set a small opcache.jit_buffer_size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dstogov Updated the jit_buffer_size to be the same in all tests.

'opcache.jit_max_side_traces=100000',
'opcache.jit_max_exit_counters=100000',
'opcache.jit_blacklist_root_trace=255',
'opcache.jit_blacklist_side_trace=255',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand the intention to increase "blacklist" settings. This will delay tracing JIT and actually disable JIT-ing for some tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dstogov Could you clarify a bit? Looking at the logic in zend_jit_trace_is_bad_root, it seemed to me that the higher the blacklist value, the less likely it is for a given trace to be blacklisted; by increasing the value, if the same trace errors out N-2 times, but then at the N-1th time gets compiled successfully, and N is the maximum allowed value (255), this should lead to an overall increase in the number of compiled traces; am I missing anything, and if yes, mind also updating the docs to clarify it a bit? :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, in some cases blacklisting of one traces may open the possibility to record and compile other traces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ok then, reverted!

'opcache.protect_memory=1',
'zend.assertions=1',
'zend.exception_ignore_args=0',
'zend.exception_string_param_max_len=15',
Expand Down