Trigger JIT tracing&compilation more often in tests #12250

danog · 2023-09-20T13:53:15Z

In my understanding, this should catch more issues (actually, running the amphp/parallel tests with this JIT config locally already causes a hang...).
I've also considered adding Psalm and PHPUnit to the nightly tests, would you be OK with that?

iluuu1994

Thanks for investigating! In terms of the specific flags, @dstogov should judge whether they make sense.

I've also considered adding Psalm and PHPUnit to the nightly tests, would you be OK with that?

We already have PHPUnit in the COMMUNITY job. Adding Psalm is ok if it doesn't unreasonably increase build-time. Although adding PHPStan would likely make more sense given its popularity.

.github/workflows/nightly.yml

danog · 2023-09-20T17:30:36Z

Sorry, meant to say phpseclib instead of phpunit :)
Sure, I can add both psalm and phpstan, just running a scan on the project itself ought to be enough to trigger eventual JIT bugs, without running the phpunit testsuites to avoid raising test times too much.

danog · 2023-09-20T17:32:18Z

Also, would you be OK with adding https://github.com/amphp/ext-uv to the amphp tests?

.github/workflows/nightly.yml

dstogov · 2023-09-20T18:18:19Z

.github/workflows/nightly.yml

+            -d opcache.jit_max_root_traces=10000000
+            -d opcache.jit_max_side_traces=10000000
+            -d opcache.jit_max_exit_counters=1000000
+            -d opcache.jit_hot_loop=1
+            -d opcache.jit_hot_func=1
+            -d opcache.jit_hot_return=1
+            -d opcache.jit_hot_side_exit=1


These are probably not the best settings, because they will trigger JIT compilation on the first function entry or loop iteration and this sometime may lead to very unnatural traces that will never executed.

Anyway, I think it make sense to start with these settings and then change them to a bit less aggressive. (e.g. 2 or 3).

Note, that these settings may increase the tests time.

I am aware they increase test times, JIT compilation on first function entry/loop iteration is precisely why I chose those values, any other value (even 2) already breaks reproduction of bugs like #12249

dstogov

Looks fine. Just remove jit_prof_threshold.

… limit

iluuu1994 · 2023-09-20T18:33:26Z

.github/workflows/nightly.yml

@@ -198,7 +205,14 @@ jobs:
            ${{ matrix.run_tests_parameters }}
            -d zend_extension=opcache.so
            -d opcache.enable_cli=1
-            -d opcache.jit_buffer_size=16M
+            -d opcache.jit_buffer_size=64M


We should probably move this to a common place, e.g. directly in .github/actions/test-linux/action.yml and enable it via parameter.

Actually, all JIT parameters can directly be moved unconditionally in .github/actions/test-linux/action.yml, and here we can just provide the -d zend_extension=opcache.so, let me do that...

iluuu1994 · 2023-09-20T18:38:14Z

@danog Can you please also make the changes in push.yml? I'd like to see how much this adds to the build time. I'm also worried it will make the COMMUNITY job much slower. It may even be counterproductive if too many things fill the cache quickly.

danog · 2023-09-20T18:51:15Z

Done!

danog · 2023-09-20T20:35:42Z

@iluuu1994 Seems the build times are OK, they're somehow even better than on master?

https://github.com/danog/php-src/actions/runs/6253638654/job/16979522694

danog · 2023-09-21T13:27:39Z

Added phpseclib, psalm and phpstan JIT nightly tests, and added ext-uv to the AMP tests since that's generally the best driver for AMP.

Also changed the JIT config in all other CIs.

iluuu1994

Thanks @danog!

.github/workflows/nightly.yml

iluuu1994 · 2023-09-22T09:43:31Z

.github/workflows/nightly.yml

+          echo opcache.jit_hot_side_exit=1 >> /etc/php.d/opcache.ini
+          echo memory_limit=-1 >> /etc/php.d/opcache.ini
+          php -v
+      - name: Test PHPSeclib


Can you please split this into a separate PR? Can you also temporarily copy this to push.yml to show that the tests are succeeding? It would be great if we could trigger this on-demand, but we don't have that currently...

Will split this to a separate PR, actually most of the new nightly tests are failing under JIT, might be worth pinging @dstogov about this:

PHPStan is triggering a JIT assertion on master (https://github.com/danog/php-src/actions/runs/6262586495/job/17005155671), opened issue Tracing JIT assertion crash when using phpstan #12262

Amphp with JIT and ext-uv is hanging on master (https://github.com/danog/php-src/actions/runs/6261267298/job/17001045071), opened issue Hang when using amphp and tracing JIT #12249

Psalm on 8.2 is triggering a tracing JIT bug (can't run the nightly 8.2 tests on my fork so don't have a link), opened issue Tracing JIT triggers wrong Array to string conversion in Psalm #12255

Split to #12270

Will split this to a separate PR, actually most of the new nightly tests are failing under JIT, might be worth pinging @dstogov about this:

Thanks. I've assigned these three issues to myself.

iluuu1994 · 2023-09-22T09:45:45Z

.github/workflows/push.yml

@@ -129,7 +129,7 @@ jobs:
          runTestsParameters: >-
            -d zend_extension=opcache.so
            -d opcache.enable_cli=1
-            ${{ !matrix.asan && '-d opcache.jit_buffer_size=16M' || '' }}
+            ${{ matrix.asan && '-d opcache.jit=disable' || '-d opcache.jit=tracing' }}


This would result in opcache.jit=tracing twice for non-asan, as it's already added in the test action.

Hmm, where? I see only two instances of opcache.jit=tracing in this workflow, one here and the other in the mac OS tests...

ext/opcache/tests/jit/bug11917.phpt

danog · 2023-09-22T10:33:56Z

btw @iluuu1994 what do you think about #12263, do you think I could add the new workflow files for RISC-V tests directly in this PR?

danog · 2023-09-29T15:37:47Z

Ping @iluuu1994, is this OK to merge? :)

iluuu1994 · 2023-09-29T16:30:35Z

@danog I'm on vacation, I'll check next week.

iluuu1994

@danog The changes look good to me, thank you! However, I would prefer merging this after the JIT issues have been fixed. A failing build is annoying, but can also hide new problems.

.github/workflows/nightly.yml

danog · 2023-10-03T08:24:58Z

Thank you @iluuu1994!
I agree that it's not a good idea to merge PRs with failing tests, even if they were not caused by that PR, but the tests in this PR are passing since I split the failing ones in another PR, why can't this be merged?

iluuu1994 · 2023-10-03T08:27:32Z

@danog From what I understand this causes the COMMUNITY build to fail at least for Psalm and amp-parallel, is that not correct?

danog · 2023-10-03T08:36:33Z

@iluuu1994 Nope, amp-parallel only fails if the uv extension is installed with JIT (and I don't install it in this PR anymore), and I didn't add Psalm this this PR.

iluuu1994 · 2023-10-03T08:59:26Z

@danog For some reason I misremembered and thought we already had Psalm in CI ^^ Thanks, I will merge this then.

Trigger JIT tracing&compilation more often, enable JIT for ASAN

ef6db9f

danog requested review from iluuu1994 and TimWolla as code owners September 20, 2023 13:53

danog changed the title ~~Trigger JIT tracing&compilation more often, enable JIT for ASAN~~ Trigger JIT tracing&compilation more often in tests, enable JIT for ASAN Sep 20, 2023

Revert some changes

4e504e1

danog changed the title ~~Trigger JIT tracing&compilation more often in tests, enable JIT for ASAN~~ Trigger JIT tracing&compilation more often in tests Sep 20, 2023

iluuu1994 reviewed Sep 20, 2023

View reviewed changes

.github/workflows/nightly.yml Outdated Show resolved Hide resolved

Fix

2c965c1

dstogov reviewed Sep 20, 2023

View reviewed changes

.github/workflows/nightly.yml Outdated Show resolved Hide resolved

dstogov reviewed Sep 20, 2023

View reviewed changes

dstogov approved these changes Sep 20, 2023

View reviewed changes

danog added 4 commits September 20, 2023 20:24

Increase JIT buffer size since Psalm get dangerously close to the 16M…

ca53391

… limit

Add test for php#11917

88cf395

Remove jit_prof_threshold

b3a2b3e

Run opcache tests with JIT on windows

a979682

iluuu1994 reviewed Sep 20, 2023

View reviewed changes

Unify JIT settings and explicitly disable JIT where needed

94714eb

Move new JIT test, fixup mac OS builds too

f397961

github-actions bot added the Extension: opcache label Sep 20, 2023

danog added 3 commits September 20, 2023 21:15

Explicitly disable JIT for ASAN builds

b4ac0e2

Remove mistakenly added additional 0

8c92c0e

Add phpseclib, psalm, phpstan nightly JIT+ASAN tests

8701969

danog mentioned this pull request Sep 20, 2023

Tracing JIT triggers wrong Array to string conversion in Psalm #12255

Closed

Change config in all other CIs

104f03f

Change config in all other CIs

c5af745

danog mentioned this pull request Sep 21, 2023

Add PHP 8.4 support phpseclib/phpseclib#1944

Closed

danog added 6 commits September 21, 2023 11:50

Parallelize

f822a56

Remove RAM limitations

4aa113f

Update

543b52b

Fix deps

a06eea9

Fix deps

8de7066

Fixup opcache

1a4d450

danog requested a review from iluuu1994 September 21, 2023 13:26

danog added 2 commits September 21, 2023 15:38

Reorder

96f7b74

fix

84373f0

danog mentioned this pull request Sep 21, 2023

Tracing JIT assertion crash when using phpstan #12262

Closed

Update to upstream phpseclib

f837597

iluuu1994 reviewed Sep 22, 2023

View reviewed changes

Improve workflow

4caf2d6

github-actions bot removed the Extension: opcache label Sep 22, 2023

Remove new tests, move to separate PR

12281a4

iluuu1994 approved these changes Oct 2, 2023

View reviewed changes

iluuu1994 reviewed Oct 2, 2023

View reviewed changes

.github/workflows/nightly.yml Outdated Show resolved Hide resolved

Remove stray line

dbd4005

iluuu1994 closed this in 098d9ca Oct 3, 2023

iluuu1994 mentioned this pull request Oct 5, 2023

JIT leak in Symfony TranslationDebugCommandTest #12364

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trigger JIT tracing&compilation more often in tests #12250

Trigger JIT tracing&compilation more often in tests #12250

danog commented Sep 20, 2023

iluuu1994 left a comment

danog commented Sep 20, 2023 •

edited

Loading

danog commented Sep 20, 2023

dstogov Sep 20, 2023

danog Sep 20, 2023 •

edited

Loading

dstogov left a comment

iluuu1994 Sep 20, 2023

danog Sep 20, 2023 •

edited

Loading

iluuu1994 commented Sep 20, 2023 •

edited

Loading

danog commented Sep 20, 2023

danog commented Sep 20, 2023 •

edited

Loading

danog commented Sep 21, 2023 •

edited

Loading

iluuu1994 left a comment

iluuu1994 Sep 22, 2023

danog Sep 22, 2023 •

edited

Loading

danog Sep 22, 2023

dstogov Oct 2, 2023

iluuu1994 Sep 22, 2023

danog Sep 22, 2023

danog commented Sep 22, 2023

danog commented Sep 29, 2023

iluuu1994 commented Sep 29, 2023

iluuu1994 left a comment

danog commented Oct 3, 2023

iluuu1994 commented Oct 3, 2023

danog commented Oct 3, 2023

iluuu1994 commented Oct 3, 2023

Trigger JIT tracing&compilation more often in tests #12250

Trigger JIT tracing&compilation more often in tests #12250

Conversation

danog commented Sep 20, 2023

iluuu1994 left a comment

Choose a reason for hiding this comment

danog commented Sep 20, 2023 • edited Loading

danog commented Sep 20, 2023

dstogov Sep 20, 2023

Choose a reason for hiding this comment

danog Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

dstogov left a comment

Choose a reason for hiding this comment

iluuu1994 Sep 20, 2023

Choose a reason for hiding this comment

danog Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

iluuu1994 commented Sep 20, 2023 • edited Loading

danog commented Sep 20, 2023

danog commented Sep 20, 2023 • edited Loading

danog commented Sep 21, 2023 • edited Loading

iluuu1994 left a comment

Choose a reason for hiding this comment

iluuu1994 Sep 22, 2023

Choose a reason for hiding this comment

danog Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

danog Sep 22, 2023

Choose a reason for hiding this comment

dstogov Oct 2, 2023

Choose a reason for hiding this comment

iluuu1994 Sep 22, 2023

Choose a reason for hiding this comment

danog Sep 22, 2023

Choose a reason for hiding this comment

danog commented Sep 22, 2023

danog commented Sep 29, 2023

iluuu1994 commented Sep 29, 2023

iluuu1994 left a comment

Choose a reason for hiding this comment

danog commented Oct 3, 2023

iluuu1994 commented Oct 3, 2023

danog commented Oct 3, 2023

iluuu1994 commented Oct 3, 2023

danog commented Sep 20, 2023 •

edited

Loading

danog Sep 20, 2023 •

edited

Loading

danog Sep 20, 2023 •

edited

Loading

iluuu1994 commented Sep 20, 2023 •

edited

Loading

danog commented Sep 20, 2023 •

edited

Loading

danog commented Sep 21, 2023 •

edited

Loading

danog Sep 22, 2023 •

edited

Loading