[opcache] [jit] Attribute support

[beberlei]

Remove the obsolete doc-comment based JIT trigger. Since function and
tracing JITs are recommended for now, the attribute based trigger mode
is removed for now.

Only @@jit("off") is available to disable the JIT for a function/method.

Open Questions:

• Are we ok with removing ´@jit("on"), @@jit("tracing")and@@jit("function")` for now to thoroughly discuss best approach for 8.1?
• Rename @@Jit to something more specific like @@JitOptions, @@JitConfig or @@JitHint?
• Remove the attribute trigger constant 4, and move tracing JIT to use 4 instead of 5?

[beberlei]

@dstogov This is my attempt at Attribute support for JIT. Looking forward for your feedback.

[nikic]

I think we need some more clarity on what we expect these attributes to actually be useful for.

Is the attribute trigger mode something purely for debugging, so one can JIT one function out of a large code base easily?

[beberlei]

@nikic I agree, @@Jit("off") works independent of trigger mode, that makes it clear. But the others only work with the attribute trigger.

More consistent would probably be to remove the trigger=attribute completly, and have the @@Jit attribute work as an overwrite that immediately JITs the attributed function. This would not make use of the tracing / callgraph optimizations though that other triggers have, potentially causing less optimized code?

[TysonAndre]

It seems inconsistent to emit E_COMPILE_ERROR with opcache but not without opcache. I'd personally prefer E_WARNING like some of the other warnings about failing to optimize.

• And to return early but proceed with optimizing the file anywhere where the attribute is seen with invalid count or argument types

ext/opcache/zend_accelerator_module.c
893:                    zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " could not compile file %s", handle.filename);

Also, zend_error proceeds with execution after being called. You may want zend_error_noreturn(E_COMPILE_ERROR, if the intent is actually to make php stop executing - I don't know if that actually works in the opcache step, though

[beberlei]

@dstogov can you comment please? Looking to merge this tomorrow morning (Europe time)

[dstogov]

looks good, but I would remove opcache.jit=attributes.
We will also have to add checks into trace recorder to stop recording for functions with inappropriate @@jit attribute.

[nikic]

I think it would be good to bring this up on the mailing list. I'm still not completely clear on what the use-case for them will be, and I'm also a bit uncomfortable with adding a global Jit class.

[beberlei]

@nikic from am APM perspective my primary use case is @@Jit("off") to make sure instrumented code is never jitted. That can certainly be handled by an internal API, but I understand Dmitry is reluctant to add one when I proposed it.

I would be willing to hold this off until 8.1 to discuss the implications and use-cases in more detail, if extensions get another way to selectively control the JIT.

The positive use case of enabling the JIT for certain functions I can see when code gets microoptimized towards the JIT, then maybe a human makes better decisions on what function should be jitted, and which ones not.

[beberlei]

@dstogov One question I just had, if I wanted to disable the JIT for selected functions:

It is now possible for an extension to use opcache.jit at runtime to disable the JIT in a zend_compile hook for a selected file and then reset to its previous value afterwards?

[beberlei]

@dstogov ping, sorry for being pushy :-(