Thank you @nicolas-grekas.

This is just an idea that came up reading this PR, but would doing this for the dumped code be a similar easy fix?

Oh, why PHP's opcode does not inline constants values? Is it possible to do something here from PHP side? // @nikic

@dkarlovi I think we already do that and not only for the container but for our own code too. Example: #24866

@Koc because the constant is defined in a separate file, and OPCache optimizations are per-file (to avoid having the optimized state depending on the list of previously loaded files)

One stupid question. Why you chose 10 000 000 times, but not 1 000 000 000 (or whatever). So it can be 1000% or 100000% faster. For me this micro optimization is useless. We just lose code quality here.

@zilionis really? Can you run the script locally with these different numbers and report back?

@zilionis I think there's some error in those numbers. If Nico's benchmarks iterate 10 million times and goes from 2s to 1s ... if we increase iterations 10x to 100 million times, I expect time to go from 20s to 10s ... so the gain remains at 100% and it doesn't increase.

In any case, we can of course be wrong, so please share your script so we can check the numbers again. Thanks!

@zilionis and for the second part of your comment, as replied on Twitter (https://twitter.com/nicolasgrekas/status/940885273587707904)

yes, it matters when this code is run by a large community that expect the framework to be as seamless as possible even in tight loops.

@nicolas-grekas @javiereguiluz ok, I am feeling stupid now. But anyway but about this micro change 100% looked to big for me. So i made benchmark (same test script). And I got just 15-25% performance increase.

@zilionis I was curious so I run the benchmarks myself (I'm using PHP 7.1). These are the numbers:

BEFORE 3.4896380901337
AFTER  1.6525621414185

So in my case it's consistent with 100% performance increase.

Sorry, but I find this stupid, now we have a freaking magic number.

(PHP 7.2 btw here, didn't try on anything else recently)

Currently running on work computer (7.0)
before 9.3259789943695
after 8.2541401386261
12.99%

@mvrhov "stupid" and "freaking" words are a bit strong. Please let's keep discussions friendly :)

And yes, you are right that this goes against the recommended practices but:

• It provides a huge performance improvement
• We add a useful comment next to the "magic number", so it's impossible to mistake

and we do it only for the code belonging to the hot path of the container, not for all places using the constants (all code dealing with compile-time logic has no reason to inline the value)

because the constant is defined in a separate file, and OPCache optimizations are per-file

It seems like this specific change might benefit more complex codebases greatly, it makes sense to try and resolve the constant even if the file containing it is not yet loaded.

7.2
➜ my_project php test.php
1.7662029266357
0.92619895935059
90.69%

IMHO 7.0 vs 7.2 (5-10 times faster!)

@stof thank you for explanation. No way to copy this constants to class, something like:

const EXCEPTION_ON_INVALID_REFERENCE = ContinerInterface::EXCEPTION_ON_INVALID_REFERENCE;

?

Is this really hot path for Symfony 4+? There isn't lot of public services nowadays.

@Koc the fact that you reference the constant from the interface has the same issue: OPCache could maybe inline the reference to the constant defined in the current file, replacing it by a direct reference to ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE (i.e. our old code). but it still does not have the value of ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE in this file (as ContainerInterface is in a different file)

Well, given that we already compile the container into an non-humanly readable mess in a generated namespace, why wouldn't we inline all of it? The compiler can look up the final value of those constants, so we can completely inline all of it. Ie.

class appLocalDebugProjectContainer extends Container
{
    const EXCEPTION_ON_INVALID_REFERENCE = 1;
    const NULL_ON_INVALID_REFERENCE = 2;
    const IGNORE_ON_INVALID_REFERENCE = 3;
    const IGNORE_ON_UNINITIALIZED_REFERENCE = 4;
   ...

Alternatively, PHP is fine with multiple namespaces in the same file, so we could even put the entire inheritance tree of the container into the compiled container file so they're all in the same opcache scope. Might get messy if the autoloader got one of them first though.

Everyone is coming with better ideas, but please run them before, the script is so simple.
Here is what your suggestion triggers:

Fatal error: Cannot inherit previously-inherited or override constant EXCEPTION_ON_INVALID_REFERENCE from interface Symfony\Component\DependencyInjection\ResettableContainerInterface in [...]

Oh I was already expecting that not to work but given stof's answer I was blindly assuming I was wrong in this haha.

Constants from the same class should already be inlined by PHP: https://3v4l.org/jM5BA/vld#output.

@kelunik not when taking inheritance into account (and the parent class is in another file.)

@kelunik these constants are not from the same class but from a parent

Class constants are now inlined at runtime.
Implemented in master: php/php-src@1a63fa6