Repository navigation
Fix GH-17770: support opcache.file_cache with JIT - #21219
gregharmon wants to merge 2 commits into
Conversation
|
Addressed the Windows CI failure in the new GH-17770 regression test. Update in
This keeps the regression intent unchanged while avoiding startup/path issues on Windows workers. |
JIT compilation takes time. And it's not going to be profitable in case we re-do it on each request. Probably, this change may be helpful for some cases - e.g rare opcache resets, but it's probably not a big problem to recompile php from sources. In case of often resets, enabling JIT will probably increase CPU load and for file_cache_only increase it significantly. |
|
@dstogov How would you like to proceed here? This seems like a reasonable change to me. File cache can be used to reduce cache stampede, this applies even when JIT is enabled. But if you're against this change, the PR should be closed. |
|
I'm not completely against this PR. |
Better, JIT should be disabled for scripts loaded from file-cache, but not loaded into shared memory. |
|
Is there some reason this can't be merged. It seems like it does the correct thing. A more comprehensive solution might be to seperate JIT metadata from the opcache opcode data entirely, this could potentially even allow on disk storage of JIT (useful for same machine, multiple invokation e.g CLI). |
|
@gregharmon I tested this against a PHP 5.6 base (unfortunately jit is pretty broken in 5.5 under our test matrix) against a variety of different tools (phan, phpstan, phpunit, etc) and internal projects. Ended up re-working this concept slightly as I was recording crashes. AI refused to review my work due to lack of Daybreak approval (working on it) so potentially it contains a defect I havent been able to spot. This patch is still under review on our end. 0001-jit-with-file-cache-v2.patch In PR form I would strip the comments but I retained for @gregharmon's understanding. |
|
Also is there some project of PHP JIT regression tests? It feels like this kind of issue should be automatedly tested. |
|
I have Daybreak, working on an update. |
3edc2b7 to
f195f22
Compare
Restore original VM handlers and clear process-local JIT metadata in the serialized copy before writing bytecode to disk. Rebuild JIT when a cached script is loaded into shared memory. Keep JIT off for process-local file cache loads. Cover disk writes and reloads across four JIT trigger modes. Co-authored-by: Mathew Heard <mheard@x4b.net>
f195f22 to
22ecf2a
Compare
Restore the original VM handler in place in the serialized copy. Specialized handlers such as ASSIGN_DIM read the next OP_DATA operand type. A stack copy of the opline does not have that neighbour, so the file cache stored an invalid handler and failed to load. Add an array assignment to the phpGH-17770 regression test.
|
😅 My PHP opcode knowledge is a bit rusty clearly. I'll give this a run through the known failures from last time. |
|
That passes against my matrix. |
Summary
Allow
opcache.file_cacheto persist scripts while JIT is enabled. The file contains bytecode and stable VM handlers; it does not contain generated machine code or process-local JIT metadata.When a file-cached script is loaded into shared memory, rebuild its JIT state. Scripts loaded into process memory continue to run without JIT, including
opcache.file_cache_onlymode.Changes
Validation
gh17770_file_cache.phpt,gh16979_check_file_cache_function.phpt, andgh18136.phpt.file_cache_frameless_handler.phptskipped locally because the minimal build did not includedl_test.