On Wed, Sep 18, 2019 at 5:10 PM Benjamin Coutu <ben.co...@zeyos.com> wrote:
> Hello,
>
> During performance testing of the (awesome) PHP 7.4 preloading feature I
> stumbled upon a lost opportunity I'd like to point out (though this has
> nothing to do with preloading per se).
>
> Please consider the following snippet:
>
> const TEST = 14;
>
> function d() {
> echo TEST;
> }
>
> The function "d" yields the following opcodes:
>
> #0 FETCH_CONSTANT "TEST" ~0
> #1 ECHO ~0
> #2 RETURN<-1> null
>
> meaning, that the constant "TEST" does not propagate into the function,
> though both, the constant and the function, are defined in the same file.
>
> Same goes for this kind of snippet:
>
> class D {
> const C = TEST;
>
> public function d() {
> echo self::C;
> }
> }
>
> Here the method "d" yields the following opcodes:
>
> #0 FETCH_CLASS_CONSTANT "C" ~0
> #1 ECHO ~0
> #2 RETURN<-1> null
>
> Interestingly enough, class constants propagate as one would expect:
>
> class D {
> const C = 14;
>
> public function d() {
> echo self::C;
> }
> }
>
> #0 ECHO 14
> #1 RETURN<-1> null
>
> I don't see why constants defined in the same file couldn't propagate
> throughout the same compilation unit during compile time, as they can never
> have a different value (locally in that particular defining file) at
> runtime.
> Is this just an oversight or is there some deeper reasoning behind this
> that I'm simply missing?
>
> In terms of preloading this would be particularly beneficial, cause one
> cloud then include/require files that define global constants before
> compiling files that contain those constants, thereby propagating their
> values throughout the code base during preloading. That would eliminate a
> lot of the runtime cost of (not so truly constant) constants.
>
> Please let me know your thoughts.
>
This optimization is unsafe, because the constant may already be previously
defined, in which case the "const" will be ignored with a notice. There is
an optimization flag that enables it, try opcache.optimization_level=-1.
Unfortunately I just noticed that I missed this notice during "engine
warning reclassification" RFC, because it occurs in zend_constants.
Otherwise we might have changed it to an exception. (Though given recent
discussions, probably not.)
Nikita
