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 9 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
37 changes: 37 additions & 0 deletions .github/patch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php
danog marked this conversation as resolved.
Show resolved Hide resolved

register_shutdown_function(function () {
$status = opcache_get_status(false);
var_dump($status);

$ok = true;
if ($status["memory_usage"]["free_memory"] < 10*1024*1024) {
danog marked this conversation as resolved.
Show resolved Hide resolved
echo "Not enough free opcache memory!".PHP_EOL;
$ok = false;
}
if ($status["interned_strings_usage"]["free_memory"] < 1*1024*1024) {
echo "Not enough free interned strings memory!".PHP_EOL;
$ok = false;
}
if ($status["jit"]["buffer_free"] < 10*1024*1024) {
echo "Not enough free JIT memory!".PHP_EOL;
$ok = false;
}
if (!$status["jit"]["on"]) {
echo "JIT is not enabled!".PHP_EOL;
$ok = false;
}

unset($status);
gc_collect_cycles();

if (!$ok) die(130);
});

$argc--;
array_shift($argv);

$_SERVER['argc']--;
array_shift($_SERVER['argv']);

require $argv[0];
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
113 changes: 106 additions & 7 deletions .github/workflows/nightly.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,66 @@ jobs:
run: |
echo zend_extension=opcache.so > /etc/php.d/opcache.ini
echo opcache.enable_cli=1 >> /etc/php.d/opcache.ini
echo opcache.enable=1 >> /etc/php.d/opcache.ini
echo opcache.protect_memory=1 >> /etc/php.d/opcache.ini
echo opcache.jit=tracing >> /etc/php.d/opcache.ini
echo opcache.jit_buffer_size=1G >> /etc/php.d/opcache.ini
echo opcache.jit_max_root_traces=100000 >> /etc/php.d/opcache.ini
echo opcache.jit_max_side_traces=100000 >> /etc/php.d/opcache.ini
echo opcache.jit_max_exit_counters=100000 >> /etc/php.d/opcache.ini
echo opcache.jit_hot_loop=1 >> /etc/php.d/opcache.ini
echo opcache.jit_hot_func=1 >> /etc/php.d/opcache.ini
echo opcache.jit_hot_return=1 >> /etc/php.d/opcache.ini
echo opcache.jit_hot_side_exit=1 >> /etc/php.d/opcache.ini
echo opcache.jit_blacklist_root_trace=255 >> /etc/php.d/opcache.ini
echo opcache.jit_blacklist_side_trace=255 >> /etc/php.d/opcache.ini
echo opcache.file_update_protection=0 >> /etc/php.d/opcache.ini
echo opcache.memory_consumption=256M >> /etc/php.d/opcache.ini
echo opcache.interned_strings_buffer=64 >> /etc/php.d/opcache.ini
echo opcache.max_accelerated_files=100000 >> /etc/php.d/opcache.ini
echo memory_limit=-1 >> /etc/php.d/opcache.ini
php -v
- name: Test Psalm
if: matrix.branch.ref != 'PHP-8.0'
run: |
git clone https://github.com/vimeo/psalm --branch=master
cd psalm
git checkout 7428e49b115a2a837aa29cf0fafd0ca902fe2457
export ASAN_OPTIONS=exitcode=139
# Needed to avoid overwriting JIT config
export PSALM_ALLOW_XDEBUG=1
php /usr/bin/composer install --no-progress --ignore-platform-reqs
php $GITHUB_WORKSPACE/.github/patch.php ./psalm --no-cache || exit $?
- name: Test PHPStan
if: matrix.branch.ref != 'PHP-8.0'
run: |
git clone https://github.com/phpstan/phpstan-src
cd phpstan-src
git checkout d02cc99d4480a203a2dbe54a5ded2da016266b11
sed -E 's/phpVersion(.*)max[^)]+/phpVersion\1max(89999/g' conf/parametersSchema.neon -i
php /usr/bin/composer install --no-progress --ignore-platform-reqs
export ASAN_OPTIONS=exitcode=139
php $GITHUB_WORKSPACE/.github/patch.php bin/phpstan clear-result-cache
php $GITHUB_WORKSPACE/.github/patch.php bin/phpstan || exit $?
- name: Test AMPHP
if: matrix.branch.ref != 'PHP-8.0'
run: |
repositories="amp cache dns file http parallel parser pipeline process serialization socket sync websocket-client websocket-server"
X=0
for repository in $repositories; do
printf "Testing amp/%s\n" "$repository"
git clone "https://github.com/amphp/$repository.git" "amphp-$repository" --depth 1
cd "amphp-$repository"
git rev-parse HEAD
php /usr/bin/composer install --no-progress --ignore-platform-reqs
export ASAN_OPTIONS=exitcode=139
php $GITHUB_WORKSPACE/.github/patch.php vendor/bin/phpunit || EXIT_CODE=$?
if [ ${EXIT_CODE:-0} -gt 128 ]; then
X=1;
fi
cd ..
done
exit $X
- name: Test Laravel
if: matrix.branch.ref != 'PHP-8.0'
run: |
Expand All @@ -343,8 +401,39 @@ jobs:
# Hack to disable a test that hangs
php -r '$c = file_get_contents("tests/Filesystem/FilesystemTest.php"); $c = str_replace("*/\n public function testSharedGet()", "* @group skip\n */\n public function testSharedGet()", $c); file_put_contents("tests/Filesystem/FilesystemTest.php", $c);'
export ASAN_OPTIONS=exitcode=139
php vendor/bin/phpunit --exclude-group skip || EXIT_CODE=$?
if [ $EXIT_CODE -gt 128 ]; then
php $GITHUB_WORKSPACE/.github/patch.php vendor/bin/phpunit --exclude-group skip || EXIT_CODE=$?
danog marked this conversation as resolved.
Show resolved Hide resolved
if [ ${EXIT_CODE:-0} -gt 128 ]; then
exit 1
fi
- name: Test ReactPHP
if: matrix.branch.ref != 'PHP-8.0'
run: |
repositories="async cache child-process datagram dns event-loop promise promise-stream promise-timer stream"
X=0
for repository in $repositories; do
printf "Testing reactphp/%s\n" "$repository"
git clone "https://github.com/reactphp/$repository.git" "reactphp-$repository" --depth 1
cd "reactphp-$repository"
git rev-parse HEAD
php /usr/bin/composer install --no-progress --ignore-platform-reqs
export ASAN_OPTIONS=exitcode=139
php $GITHUB_WORKSPACE/.github/patch.php vendor/bin/phpunit || EXIT_CODE=$?
if [ $[EXIT_CODE:-0} -gt 128 ]; then
X=1;
fi
cd ..
done
exit $X
- name: Test Revolt PHP
if: matrix.branch.ref != 'PHP-8.0'
run: |
git clone https://github.com/revoltphp/event-loop.git --depth=1
cd event-loop
git rev-parse HEAD
php /usr/bin/composer install --no-progress --ignore-platform-reqs
export ASAN_OPTIONS=exitcode=139
php $GITHUB_WORKSPACE/.github/patch.php vendor/bin/phpunit || EXIT_CODE=$?
if [ ${EXIT_CODE:-0} -gt 128 ]; then
exit 1
fi
- name: Test Symfony
Expand All @@ -363,12 +452,22 @@ jobs:
export SYMFONY_DEPRECATIONS_HELPER=max[total]=999
X=0
for component in $(find src/Symfony -mindepth 2 -type f -name phpunit.xml.dist -printf '%h\n'); do
php ./phpunit $component --exclude-group tty,benchmark,intl-data,transient --exclude-group skip || EXIT_CODE=$?
if [ $EXIT_CODE -gt 128 ]; then
php $GITHUB_WORKSPACE/.github/patch.php ./phpunit $component --exclude-group tty,benchmark,intl-data,transient --exclude-group skip || EXIT_CODE=$?
if [ ${EXIT_CODE:-0} -gt 128 ]; then
X=1;
fi
done
exit $X
- name: Test PHPSeclib
if: always()
run: |
git clone https://github.com/phpseclib/phpseclib --branch=master
cd phpseclib
git checkout 259bd9f1e8af11726ed74acf527c2c046549061b
danog marked this conversation as resolved.
Show resolved Hide resolved
export ASAN_OPTIONS=exitcode=139
export PHPSECLIB_ALLOW_JIT=1
php /usr/bin/composer install --no-progress --ignore-platform-reqs
php $GITHUB_WORKSPACE/.github/patch.php vendor/bin/paratest --verbose --configuration=tests/phpunit.xml --runner=WrapperRunner || exit $?
- name: Test PHPUnit
if: always()
run: |
Expand All @@ -377,8 +476,8 @@ jobs:
git rev-parse HEAD
export ASAN_OPTIONS=exitcode=139
php /usr/bin/composer install --no-progress --ignore-platform-reqs
php ./phpunit || EXIT_CODE=$?
if [ $EXIT_CODE -gt 128 ]; then
php $GITHUB_WORKSPACE/.github/patch.php ./phpunit || EXIT_CODE=$?
if [ ${EXIT_CODE:-0} -gt 128 ]; then
exit 1
fi
- name: 'Symfony Preloading'
Expand All @@ -401,7 +500,7 @@ jobs:
sed -i 's/youremptytestdbnamehere/test/g' wp-tests-config.php
sed -i 's/yourusernamehere/root/g' wp-tests-config.php
sed -i 's/yourpasswordhere/root/g' wp-tests-config.php
php vendor/bin/phpunit || EXIT_CODE=$?
php $GITHUB_WORKSPACE/.github/patch.php vendor/bin/phpunit || EXIT_CODE=$?
if [ $EXIT_CODE -gt 128 ]; then
exit 1
fi
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