Rename zend-test to zend_test

[nikic]

The extension name should match the name of the ext/ directory,
otherwise it will not get picked up by run-tests. It would be possible
to remap this in run-tests, but I think it's better to rename the
extension to follow the standard format. Other extensions also
use underscore instead of hyphen (e.g. pdo_mysql and not pdo-mysql).
Of course, ./configure options are always hyphenated.

[nikic]

Ugh, there are some observer test failures when JIT is enabled (cc @SammyK)

[nikic]

I think all the test failures have the same root cause though. The "get return value even if not used" part doesn't work under JIT.

[nikic]

Okay, the reason why those tests fail is actually not related to the JIT (though I believe that problem also exists), but to the fact that opcache inlines calls to functions that contain only return constant;.

My question would then be what the expected behavior is in that case. If ZEND_OBSERVER_ENABLED, should this optimization be disabled? Or should rather optimizations be disabled only for the tests (under the assumption that the observer should only see calls that would normally happen as well)?

[morrisonlevi]

At what point does opcache do this optimization? Can it know if the function is being observed before inlining?

If not, then I think it would be ideal for extensions to disable this optimization, and not just do it unconditionally whenever an observer is attached. This is because for APM you wouldn't want to disable this optimization (probably), but for a debugger or some other kind of debugger observer you might, so let the observer choose.

[nikic]

At what point does opcache do this optimization?

php-src/ext/opcache/Optimizer/optimize_func_calls.c

Line 95 in f0d6151

static void zend_try_inline_call(zend_op_array *op_array, zend_op *fcall, zend_op *opline, zend_function *func)

Can it know if the function is being observed before inlining?

As that is only determined on first call, no.

If not, then I think it would be ideal for extensions to disable this optimization, and not just do it unconditionally whenever an observer is attached. This is because for APM you wouldn't want to disable this optimization (probably), but for a debugger or some other kind of debugger you might, so let the observer choose.

Yes, I think I agree your reasoning here. In that case I'd tag the relevant tests with opcache.optimization_level=0.

[nikic]

Tests should be fixed by dd7d829 and fbd8e20.

[nikic]

Tests pass on azure now, but fail on windows due to path separators.

[SammyK]

Thanks for the investigation and fix on this @nikic. 🙇

[mvorisek]

What about renaming extension names Zend OPcache and PDO_OCI too?

[nikic]

Renaming those would break extension_loaded checks, doesn't seem worthwhile. (The same applies to zend_test, but as it's an internal testing extension, it does not matter.)