Provide script to TSSA build in tracing JIT

[nielsdos]

For the following script:

final class Foo {
  public $prop = 0;
}
function test(Foo $obj) {
  $obj->prop=1;
}

$foo = new Foo;
for ($i=0;$i<3;$i++) {
  test($foo);
}

When comparing the TSSA (via opcache.jit_debug) vs the opcache SSA (via opcache.opt_debug_level=0x400000) we note that in TSSA, the RECV op for test does not infer the type of the argument to be class Foo. This is because the optimizer uses the script pointer to figure out known classes but TSSA always sets script to NULL. This in turn generates suboptimal assembly because zend_may_throw returns 1 due to the unknown CE in the TSSA, resulting in an extra exception check in the assembly code.

[arnaud-lb]

Possibly this can fetch a different version of the script (e.g. if the file was discarded and recompiled).

An alternative would be to store the script into zend_jit_op_array_trace_extension. (After op_array, as I believe the func_info and op_array fields must be at the same offset in all zend_jit_*_extension structs.)

[dstogov]

So simple? I wonder, why I didn't use SHM stored script before. (I can't remember any related problems).

This also relates to the fact that for tacing JIT we use opcache.jit=1254 instead of 1255, but in case we can use the SHM stored version of the script , we may consider switching to 1255 by default. @nielsdos Please double check this.

@arnaud-lb script caching and trace compilations are serialized through the same mutex. The only possible race condition I see, is script modification, between start of tracing start and start of trace compilation. That mean the new compiled trace is going to be already outdated. I think this should be fixed, by adding some check right after zend_shared_alloc_lock in zend_jit_compile_root/side_trace(). Then this solution should be fine. Do you see other problems?

[arnaud-lb]

@dstogov an other possible race condition is script modification while it's being executed (start executing script, re-compile script, start tracing) :

// main.php
require 'inc.php';
opcache_invalidate('inc.php', true);
require 'inc.php';
f();

// inc.php
if (!function_exists('f')) {
    function f() {
        for ($i = 0; $i < 200; $i++) {
            var_dump($i);
        }
    }
}

In this case we start tracing the loop when the script is already invalidated. I think your solution will handle this too, but we could avoid tracing entirely by adding a check earlier.

[dstogov]

Right! The early check is preferred.
We may link op_array to script (as you suggested) and then check that the script is not ivalidated.
Or we may use the last script from cache (link @nielsdos do) and check that the op_array belongs to it.

[dstogov]

The last check may be done using address range comparison.

[iluuu1994]

This also relates to the fact that for tacing JIT we use opcache.jit=1254 instead of 1255, but in case we can use the SHM stored version of the script , we may consider switching to 1255 by default.

I'm confused about this part. ZEND_JIT_LEVEL_OPT_SCRIPT is used from zend_persist.c, where it calls zend_jit_script() rather than zend_jit_op_array() for each individual function. zend_jit_script() will then build the call map which calls ZEND_SET_FUNC_INFO.

However, we'll then loop over the op_arrays and call zend_jit_op_array(), then zend_jit_setup_hot_trace_counters(), which will immediately install a new func info, discarding the previously collected call graph, so it doesn't seem like the tracing JIT would benefit from this flag. Do we need to rebuild the call graph for the TSSA? Would we even benefit from this, considering we'll only collect calls to other functions, which would be easy to infer locally?

[dstogov]

OK. I think you are right.