On Fri, Aug 2, 2024, at 18:51, Ilija Tovilo wrote:
> Hi everyone
>
> As you probably know, a common performance optimization in PHP is to
> prefix global function calls in namespaced code with a `\`. In
> namespaced code, relative function calls (meaning, not prefixed with
> `\`, not imported and not containing multiple namespace components)
> will be looked up in the current namespace before falling back to the
> global namespace. Prefixing the function name with `\` disambiguates
> the called function by always picking the global function.
>
> Not knowing exactly which function is called at compile time has a
> couple of downsides to this:
>
> * It leads to the aforementioned double-lookup.
> * It prevents compile-time-evaluation of pure internal functions.
> * It prevents compiling to specialized opcodes for specialized
> internal functions (e.g. strlen()).
> * It requires branching for frameless functions [1].
> * It prevents an optimization that looks up internal functions by
> offset rather than by name [2].
> * It prevents compiling to more specialized argument sending opcodes
> because of unknown by-value/by-reference passing.
>
> All of these are enabled by disambiguating the call. Unfortunately,
> prefixing all calls with `\`, or adding a `use function` at the top of
> every file is annoying and noisy. We recently got a feature request to
> change how functions are looked up [3]. The approach that appears to
> cause the smallest backwards incompatibility is to flip the order in
> which functions are looked up: Check in global scope first, and only
> then in local scope. With this approach, if we can find a global
> function at compile-time, we know this is the function that will be
> picked at run-time, hence automatically enabling the optimizations
> above. I created a PoC implementing this approach [4].
>
> Máté has kindly benchmarked the patch, measuring an improvement of
> ~3.9% for Laravel, and ~2.1% for Symfony
> (https://gist.github.com/kocsismate/75be09bf6011630ebd40a478682d6c17).
> This seems quite significant, given that no changes were required in
> either of these two codebases.
So, what you’re saying is that symfony and laravel can get a performance
increase by simply adding a \ in the right places? Why don’t they do that
instead of changing the language?
>
> There are a few noteworthy downsides:
>
> * Unqualified calls to functions in the same namespace would be
> slightly slower, because they now involve checking global scope first.
> I believe that unqualified, global calls are much more common, so this
> change should still result in a net positive. It's also possible to
> avoid this cost by adding a `use function` to the top of the file.
For functions/classes in the same exact namespace, you don’t need a use
statement. But after this change, you do in certain cases?
namespace Foo;
function array_sum($bar) {}
function baz($bar) {
return array_sum($bar);
}
So, how do you use that function in the same file?
> * Introducing new functions in the global namespace could cause a BC
> break for unqualified calls, if the function happens to have the same
> name. This is unfortunate, but likely rare. Since new functions are
> only introduced in minor/major versions, this should be manageable,
> but must be considered for every PHP upgrade.
We can only see open source code when doing impact analysis. This means picking
even a slightly “popular” name could go very poorly.
> * Some mocking libraries (e.g. Symfony's ClockMock [5]) intentionally
> declare functions called from some file in the files namespace to
> intercept these calls. This use-case would break. That said, it is
> somewhat of a fragile approach to begin with, given that it wouldn't
> work for fully qualified calls, or unnamespaced code.
See above. I’ve seen this “trick” used on many closed source projects. I’ve
also seen it used when PHP has a bug and the workaround is to implement it in
php like this.
>
> I performed a small impact analysis [6]. There are 484 namespaced
> functions shadowing global, internal functions in the top 1000
> composer packages. However, the vast majority (464) of these functions
> come from thecodingmachine/safe, whose entire purpose is offering
> safer wrappers around internal functions. Excluding this library,
> there are only 20 shadowing functions, which is surprisingly little.
> Furthermore, the patch would have no impact on users of
> thecodingmachine/safe, only on the library code itself.
>
> As for providing a migration path: One approach might be to introduce
> an INI setting that performs the function lookup in both local and
> global scope at run-time, and informs the user about the behavioral
> change in the future. To mitigate it, an explicit `use function` would
> need to be added to the top of the file, or the call would need to be
> prefixed with `namespace\`. The impact analysis [6] also provides a
> script that looks for shadowing functions in your project. It does not
> identify uses of these functions (yet), just their declarations.
>
> Lastly, I've already raised this idea in the PHP Foundations internal
> chat but did not receive much positive feedback, mostly due to fear of
> the potential BC impact. I'm not particularly convinced this is an
> issue, given the impact analysis. Given the surprisingly large
> performance benefits, I was inclined to raise it here anyway. It also
> sparked some related ideas, like providing modules that lock
> namespaces and optimize multiple files as a singular unit. That said,
> such approaches would likely be significantly more complex than the
> approach proposed here (~30 lines of C code).
>
> Anyway, please let me know about possible concerns, broken use-cases,
> or any alternative approaches that may come to mind. I'm looking
> forward to your feedback.
>
> Ilija
>
> [1] https://github.com/php/php-src/pull/12461
> [2] https://github.com/php/php-src/pull/13634
> [3] https://github.com/php/php-src/issues/13632
> [4] https://github.com/php/php-src/pull/14529
> [5]
> https://github.com/symfony/symfony/blob/7.1/src/Symfony/Bridge/PhpUnit/ClockMock.php
> [6] https://gist.github.com/iluuu1994/4b83481baac563f8f0d3204c697c5551
>
— Rob
