Re: [PHP-DEV] [RFC] Autoloader Classmap
> On 6 Apr 2021, at 05:05, Mark Randall wrote: > > On 15/03/2021 17:41, Mark Randall wrote: >> I would like to propose the addition of a new mechanism of autoloading >> classes - a classmap that will be consulted prior to checking the >> spl_autoload_register'd callbacks. >> https://wiki.php.net/rfc/autoload_classmap > > Does anyone else have any more feedback on this? If not I plan on opening > voting in a couple of weeks or so. > > The tl;dr: > > * Autoloading is one of the routines called most frequently in any request. > > * It's a very minor boost in autoloading performance, around 5% vs invoking a > userland function. This will easily be swamped by any IO and invalidated > entirely by preloading. > > * I expect 99.% of users will never know it exists, and it will instead > just be an option for tools like composer that will provide a small > transparent boost. > > * It provides a very minor benefit to debugging as you get to skip over the > autoloading frames which so very often come up during a request. > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > Hi Mark, Just to clarify something - wouldn't the loader use ‘require’ behaviour, not ‘require_once’ behaviour? If you’re hitting the autoloader, that means the class in the file you’re about to load isn’t defined yet, which means the file hasn’t been loaded already. Using require_once you’d be doing a check if the file has loaded in a scenario where it would have never been loaded anyway. Cheers Stephen -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
Le 20/03/2021 à 02:59, Mike Schinkel a écrit : - It could make XDEBUG debugging much less tedious by eliminating the drill-down into the autoloader every time an unloaded class is referenced. Yes ! I second that, it would be a huge win for a lot of overly pessimistic developers that do xdebug-driven development such as me. Regards, -- Pierre -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
> On Mar 19, 2021, at 2:40 PM, Mark Randall wrote: > > On 19/03/2021 14:45, Nikita Popov wrote: >> Could you please update the RFC with some performance numbers, including >> the used methodology? The number of 5% has been floating around, but it's >> not clear what it refers to. I'm primarily interested in end-to-end effect, >> e.g. on the symfony demo project. > > I have updated the RFC with the following: > > Testing suggests that autoloading through an internal classmap delivers > around 5% performance increase vs a userland function call (e.g. composer). > > This is 5% of the cost of the autoloading, and not the execution as a whole. > Testing was performed by creating 50,000 empty classes each in an individual > file, and then autoloading every one of them in a loop. Amount of classes was > purely to help reduce noise. > > Average for internal classmap was 0.295 vs 0.313 for spl_autoload_register > representing 5.9% difference (in autoload performance only). > > >> > > I appreciate this is a small amount. If internals feels the additional code > is not worth it for such a small gain, I have no issue at all with the RFC > being rejected. JMTCW, but I see a gain for this proposal that is more than just performance. - It allows use-cases that don't fit PSR-4 — like WordPress plugins — to opt-in to using a core autoloader w/o having roll their own and end up with having tens or more registered autoloaders. - It could make XDEBUG debugging much less tedious by eliminating the drill-down into the autoloader every time an unloaded class is referenced. #fwiw -Mike -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
Please ignore my last message :(
Hi Internals,
I did a somewhat rough test to compare Composer's autoloader and the
function proposed in the RFC.
## TL;DR:
autoload_set_classmap over Composer with OPCache: 8.12%
autoload_set_classmap over Composer without OPCache: 7.93%
## Long version:
Using symfony/framework-bundle , symfony/console, and phpunit/phpunit,
in a new composer project, I created a classmap (`composer
dump-autoload -ao`) in `vendor/composer/autoload_classmap.php`, file.
To keep the test fair, I removed the Composer autoload.files (always
include those files), and platform-check to keep the test fair. I had
to comment out the "Error during autoloading from classmap. Entry
\"%s\" failed to load the class from \"%s\" (Class undefined after
file included)" exception because it errored for valid classes. Using
Composer 2.
Benchmark called `class_exists()` on the Composer-generated classmap,
with autoloading enabled. On the same tests, it called 10K
`class_exists('test\\test_' . $i)` where $i = 1...10K. Results are
averaged from 3 runs.
**Composer autoloader**
With OPCache: 20.337 sec.
Without OPCache: 20.042 sec.
**autoload_set_classmap**
With OPCache: 18.684 sec
Without OPCache: 18.451 sec
**Performance gain**
autoload_set_classmap over Composer with OPCache: 8.12%
autoload_set_classmap over Composer without OPCache: 7.93%
I think 7-8% difference is more of a best-case scenario for
autoload_set_classmap, because the default unauthoritative classmap
has PSR-4/0/files/classmap features, and some sanity checks to avoid
collisions. I don't think a reasonable PHP application will have
2K autoload hits and 10K misses. This is indeed an improvement, and
something I would use myself if approved. However, I doubt real-life
applications would see any noticeable difference in other measures
such as response time or req/sec count.
Smile,
Ayesh.
On Fri, Mar 19, 2021 at 8:16 PM Nikita Popov wrote:
>
> On Mon, Mar 15, 2021 at 6:41 PM Mark Randall wrote:
>
> > Hi Internals,
> >
> > I would like to propose the addition of a new mechanism of autoloading
> > classes - a classmap that will be consulted prior to checking the
> > spl_autoload_register'd callbacks.
> >
> > https://wiki.php.net/rfc/autoload_classmap
>
>
> Could you please update the RFC with some performance numbers, including
> the used methodology? The number of 5% has been floating around, but it's
> not clear what it refers to. I'm primarily interested in end-to-end effect,
> e.g. on the symfony demo project.
>
> Regards,
> Nikita
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
Hi Internals,
I did a somewhat rough test to compare Composer's autoloader and the
function proposed in the RFC.
## TL;DR:
Composer autoloader + Opcache:
autoload_set_classmap + Opcache:
## Long version:
Using symfony/framework-bundle , symfony/console, and phpunit/phpunit,
in a new composer project, I created a classmap (`composer
dump-autoload -ao`) in `vendor/composer/autoload_classmap.php`, file.
To keep the test fair, I removed the Composer autoload.files (always
include those files), and platform-check to keep the test fair. I had
to comment out the "Error during autoloading from classmap. Entry
\"%s\" failed to load the class from \"%s\" (Class undefined after
file included)" exception because it errored for valid classes. Using
Composer 2.
Benchmark called `class_exists()` on the Composer-generated classmap,
with autoloading enabled. On the same tests, it called 10K
`class_exists('test\\test_' . $i)` where $i = 1...10K. Results are
averaged from 3 runs.
**Composer autoloader**
With OPCache: 20.337 sec.
Without OPCache: 20.042 sec.
**autoload_set_classmap**
With OPCache: 18.684 sec
Without OPCache: 18.451 sec
**Performance gain**
autoload_set_classmap over Composer with OPCache: 8.12%
autoload_set_classmap over Composer without OPCache: 7.93%
I think 7-8% difference is more of a best-case scenario for
autoload_set_classmap, because the default unauthoritative classmap
has PSR-4/0/files/classmap features, and some sanity checks to avoid
collisions. That, I don't think a reasonable PHP application will have
2K autoload hits and 10K misses. This is indeed an improvement, and
something I would use myself if approved. However, I doubt real-life
applications would see any noticeable difference in other measures
such as response time or req/sec count.
Smile,
Ayesh.
On Fri, Mar 19, 2021 at 8:16 PM Nikita Popov wrote:
>
> On Mon, Mar 15, 2021 at 6:41 PM Mark Randall wrote:
>
> > Hi Internals,
> >
> > I would like to propose the addition of a new mechanism of autoloading
> > classes - a classmap that will be consulted prior to checking the
> > spl_autoload_register'd callbacks.
> >
> > https://wiki.php.net/rfc/autoload_classmap
>
>
> Could you please update the RFC with some performance numbers, including
> the used methodology? The number of 5% has been floating around, but it's
> not clear what it refers to. I'm primarily interested in end-to-end effect,
> e.g. on the symfony demo project.
>
> Regards,
> Nikita
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On 19/03/2021 14:45, Nikita Popov wrote: Could you please update the RFC with some performance numbers, including the used methodology? The number of 5% has been floating around, but it's not clear what it refers to. I'm primarily interested in end-to-end effect, e.g. on the symfony demo project. I have updated the RFC with the following: Testing suggests that autoloading through an internal classmap delivers around 5% performance increase vs a userland function call (e.g. composer). This is 5% of the cost of the autoloading, and not the execution as a whole. Testing was performed by creating 50,000 empty classes each in an individual file, and then autoloading every one of them in a loop. Amount of classes was purely to help reduce noise. Average for internal classmap was 0.295 vs 0.313 for spl_autoload_register representing 5.9% difference (in autoload performance only). >> I appreciate this is a small amount. If internals feels the additional code is not worth it for such a small gain, I have no issue at all with the RFC being rejected. Mark Randall -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Mon, Mar 15, 2021 at 6:41 PM Mark Randall wrote: > Hi Internals, > > I would like to propose the addition of a new mechanism of autoloading > classes - a classmap that will be consulted prior to checking the > spl_autoload_register'd callbacks. > > https://wiki.php.net/rfc/autoload_classmap Could you please update the RFC with some performance numbers, including the used methodology? The number of 5% has been floating around, but it's not clear what it refers to. I'm primarily interested in end-to-end effect, e.g. on the symfony demo project. Regards, Nikita
Re: [PHP-DEV] [RFC] Autoloader Classmap
Am 17.03.2021 um 02:51 schrieb Mark Randall : > It will be left to the voters to decide if 5% of an extremely common > operation is worth the few dozen lines of code required. Just to avoid misunderstanding things: This is a 5% boost of just the autoloading time, right? What does this translate to - before vs. after, e.g. in ms - for a real-world project, everything included? I'm still undecided if I'm in the this-is-not-worth-it or the it-is-simple-enough-to-add camp. - Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On 17/03/2021 01:29, Mike Schinkel wrote: Would core not be able to implement an add() more efficiently than a get()/set()? I am especially concerned the get()/set() approach might require allocating memory for the returned array for every get(). There is no good way to add features to mutate the existing array without causing a net performance penalty considering the approximately 5% boost this RFC provides vs a userland function. It is a micro-optimization written exclusively with a single large pre-generated list in mind. It will be left to the voters to decide if 5% of an extremely common operation is worth the few dozen lines of code required. Mark Randall -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
> On Mar 16, 2021, at 1:46 PM, Levi Morrison via internals
> wrote:
>
> On Tue, Mar 16, 2021 at 11:06 AM Harm Smits wrote:
>>
>> Hey,
>>
>> First time ever replying so hope all went well.
>>
>> I believe the `autoload_set_classmap` function's signature should change
>> rather to something like this:
>>
>> ```
>> /**
>> * Sets the internal autoloader to use the given classmap.
>> *
>> * @param $mapping key value pair of classes;
>> * @param $merge A boolean value indicating whether it should *merge* with
>> an already defined classmap or override in its interity;
>> * @param $merge_override Whether existing keys in the classmap will be
>> overridden.
>> *
>> * @return void
>> */
>> autoload_set_classmap(array $mapping, bool $merge = true, bool
>> $merge_override = false): void {}
>> ```
>>
>> It makes more sense as some projects might have multiple composer projects
>> loaded (think about modules in e.g. wordpress), where there can be multiple
>> autoloaders. Hence the *$merge* and *$merge_override* variables. I don't
>> know whether we should rather use flags for this, but considering named
>> arguments are here now, I'd say it makes no sense to use them anymore
>> (except for maybe consistency). Not sure about the implementation, but the
>> features should be there for sure.
>>
>> Kind regards,
>> Harm Smits
>
> The proposed API allows users to fetch the current map and set a new
> map. In other words, the callers can choose these decisions among
> others (merge some keys, override others).
I assume what you are proposing as an solution is:
$classmap = autoload_get_classmap();
$classmap["Foo"] = "/path/to/Foo.php";
autoload_set_classmap($classmap);
Instead of adding a third function to allow this:
autoload_add_classmap( [ "Foo" => "/path/to/Foo.php" ]);
Did I assume correctly?
If yes, then a question (because I do not know enough of about how internal
data structure handling works in PHP):
Would core not be able to implement an add() more efficiently than a
get()/set()? I am especially concerned the get()/set() approach might require
allocating memory for the returned array for every get().
If both of those approaches would be equivalent from a memory and/or
performance perspective then a get()/set() could be sufficient. But if there is
a significant difference in memory use and/or performance then ideally we need
an add() (or register()) too, no?
-Mike
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
> On 17 Mar 2021, at 03:11, Jordi Boggiano wrote: > > > On 16/03/2021 15:02, Stephen Reay wrote: >>> On 16 Mar 2021, at 20:07, Jordi Boggiano wrote: >>> >>> >>> P.S: While I am here looking at spl_* docs, it seems to me like >>> spl_autoload_call should be deprecated in favor of class_exists, and >>> spl_autoload_extensions + spl_autoload also probably should be deprecated >>> or at least marked as antiquated in the docs, and not migrated to >>> autoload_* like you suggested doing for spl_autoload_register/unregister in >>> another RFC. >>> >>> >> >> Edit: wrong sending address, again. Sending for list, apologies for the dupe. >> >> I don’t have a huge amount of interest in this, except to say: If someone >> wants to add a core function to make Composer’s autoloader work faster, >> that’s fine and dandy, it doesn’t hurt to provide the building blocks for >> common patterns. But pushing your own biases about non-Composer class >> autoloading being “antiquated” or “deprecated” is a step too far. > > I assume that refers to my P.S. note. > > I did not mean to offend anyone, I merely have never seen anyone use these > before. It seems like this relies on include_path being set and then having > big dirs full of class files that now become autoloadable, but that doesn't > play well with namespaces and doesn't let you organize stuff very much unless > you set every dir in the include_path. Call me biased but that does not seem > very user friendly. > > Anyway this is completely unrelated to the RFC at hand, it was a side note, > so let's leave it at that I guess. > > -- > > Jordi Boggiano > @seldaek - https://seld.be > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > It’s not about “offending” anyone. It’s about the inappropriateness of the author of a userland library that includes an autoload component, calling for the removal/docu-shunning of a built-in autoload function, with apparently zero actual knowledge of how it works, to boot. `spl_autoload` works just fine with namespaces, and files organised into subdirectories (under an include_path entry) mapped to the namespace names. It has worked that way since 5.3.3 - over a decade ago. Your comments about this are both factually wrong, and inappropriate, given your “role”. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On 16/03/2021 15:02, Stephen Reay wrote: On 16 Mar 2021, at 20:07, Jordi Boggiano wrote: P.S: While I am here looking at spl_* docs, it seems to me like spl_autoload_call should be deprecated in favor of class_exists, and spl_autoload_extensions + spl_autoload also probably should be deprecated or at least marked as antiquated in the docs, and not migrated to autoload_* like you suggested doing for spl_autoload_register/unregister in another RFC. Edit: wrong sending address, again. Sending for list, apologies for the dupe. I don’t have a huge amount of interest in this, except to say: If someone wants to add a core function to make Composer’s autoloader work faster, that’s fine and dandy, it doesn’t hurt to provide the building blocks for common patterns. But pushing your own biases about non-Composer class autoloading being “antiquated” or “deprecated” is a step too far. I assume that refers to my P.S. note. I did not mean to offend anyone, I merely have never seen anyone use these before. It seems like this relies on include_path being set and then having big dirs full of class files that now become autoloadable, but that doesn't play well with namespaces and doesn't let you organize stuff very much unless you set every dir in the include_path. Call me biased but that does not seem very user friendly. Anyway this is completely unrelated to the RFC at hand, it was a side note, so let's leave it at that I guess. -- Jordi Boggiano @seldaek - https://seld.be -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Tue, Mar 16, 2021 at 11:06 AM Harm Smits wrote:
>
> Hey,
>
> First time ever replying so hope all went well.
>
> I believe the `autoload_set_classmap` function's signature should change
> rather to something like this:
>
> ```
> /**
> * Sets the internal autoloader to use the given classmap.
> *
> * @param $mapping key value pair of classes;
> * @param $merge A boolean value indicating whether it should *merge* with
> an already defined classmap or override in its interity;
> * @param $merge_override Whether existing keys in the classmap will be
> overridden.
> *
> * @return void
> */
> autoload_set_classmap(array $mapping, bool $merge = true, bool
> $merge_override = false): void {}
> ```
>
> It makes more sense as some projects might have multiple composer projects
> loaded (think about modules in e.g. wordpress), where there can be multiple
> autoloaders. Hence the *$merge* and *$merge_override* variables. I don't
> know whether we should rather use flags for this, but considering named
> arguments are here now, I'd say it makes no sense to use them anymore
> (except for maybe consistency). Not sure about the implementation, but the
> features should be there for sure.
>
> Kind regards,
> Harm Smits
The proposed API allows users to fetch the current map and set a new
map. In other words, the callers can choose these decisions among
others (merge some keys, override others).
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Mon, Mar 15, 2021 at 12:41 PM Mark Randall wrote:
> I would like to propose the addition of a new mechanism of autoloading
> classes - a classmap that will be consulted prior to checking the
> spl_autoload_register'd callbacks.
>
> https://wiki.php.net/rfc/autoload_classmap
>
>
My initial reaction is (can you guess?): This can be done in userspace.
function autoload_classmap(array $classes): void {
spl_autoload_register(fn(string $class): void {
if (isset($classes[$class])) require($classes[$class]);
});
}
This is essentially what composer already does for class maps, and it seems
to work quite well there.
However, I can see some small performance benefit from not having to invoke
a userspace function (very small), so it's not wholly without merit.
Where I have more issue is with case folding. I see three options:
1/ Require users to treat their class names as case-sensitive if they're
going to use a class map. Personally, I like that this forces users away
from case-insensitivity without breaking existing apps.
2/ Require users to pass a map with folded classnames. I expect the
significant majority of users of this are using machine generated maps, so
doing the case fold at "compile" time should be trivial.
3/ Allow any case in the map and fold it to lower at set time. This is the
worst option IMO and no matter how it's implemented it's going to negate
any performance gain it introduces. If someone *REALLY* wants that, they
can map their array before invoking autoload_classmap(). This also forces
them to make a decision about conflict resolution themselves.
-Sara
Re: [PHP-DEV] [RFC] Autoloader Classmap
Hey,
First time ever replying so hope all went well.
I believe the `autoload_set_classmap` function's signature should change
rather to something like this:
```
/**
* Sets the internal autoloader to use the given classmap.
*
* @param $mapping key value pair of classes;
* @param $merge A boolean value indicating whether it should *merge* with
an already defined classmap or override in its interity;
* @param $merge_override Whether existing keys in the classmap will be
overridden.
*
* @return void
*/
autoload_set_classmap(array $mapping, bool $merge = true, bool
$merge_override = false): void {}
```
It makes more sense as some projects might have multiple composer projects
loaded (think about modules in e.g. wordpress), where there can be multiple
autoloaders. Hence the *$merge* and *$merge_override* variables. I don't
know whether we should rather use flags for this, but considering named
arguments are here now, I'd say it makes no sense to use them anymore
(except for maybe consistency). Not sure about the implementation, but the
features should be there for sure.
Kind regards,
Harm Smits
Re: [PHP-DEV] [RFC] Autoloader Classmap
On 16/03/2021 14:21, Larry Garfield wrote: 1) As the stated reason for this RFC is performance, have you any benchmarks to show how much we'd win by using an engine-space lookup rather than user-space lookup? Performance benefit is unfortunately quite small, my tests suggest it is in the region of 3 to 5% vs invoking a userland function an equivilent number of times and looking up in a classmap. It's ever so slightly higher if comparing against something doing a lowercase on each one. If that's enough to justify it passing an RFC is left to the voters. The intent is for a free micro optimization at framework level that most users would never see. 3) The lower-casing of class names feels weird to me. Why? Given that most filesystems are case sensitive, that seems like an odd requirement and the RFC doesn't include an explanation of why that's beneficial. The filesystem is case sensitive, but the class names are not, and are handled as lowercase at engine level. new Foo() is the same - new foo() as far as lookups. Requiring lowercase names means that the lookup will perform the same regardless of which case is used in the code. Doing otherwise would require either an exact case match (which is not how PHP operates), or forcing the list to lowercase at the call to autoload_set_classmap, which would harm performance when it's perfectly reasonable to expect a static lowercase map to be given. It is important to note that only the array key is lowercase, the value, the path name, IS case sensitive. Mark Randall -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Tue, Mar 16, 2021 at 8:02 AM Mark Randall wrote: > On 15/03/2021 22:18, Levi Morrison via internals wrote: > > Like any other case insensitive symbol PHP works with that comes from > > userland, I would lower it in the engine's side of things. I would not > > push the lowercase requirement on the API. > > That was the first design, however it required re-creating the array > with potentially many thousands of new strings which slowed it down > considerably. > > As I envisenge this to be used with machine generated data, having it > use lowercase would not be an issue. > If lower case key names weren't a requirement for the user, but the engine worked on lower case keys, how would something like this behave: $map['myClass'] = '/path/to/lib/myclass1.php' $map['myclass'] = '/path/to/lib/myclass2.php' > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > -- Chase Peeler chasepee...@gmail.com
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Mon, Mar 15, 2021, at 12:41 PM, Mark Randall wrote: > Hi Internals, > > I would like to propose the addition of a new mechanism of autoloading > classes - a classmap that will be consulted prior to checking the > spl_autoload_register'd callbacks. > > https://wiki.php.net/rfc/autoload_classmap > > Mark Randall > marand...@php.net Hi Mark. A few questions. 1) As the stated reason for this RFC is performance, have you any benchmarks to show how much we'd win by using an engine-space lookup rather than user-space lookup? 2) "Setting the classmap can be performed once per request." That's not consistent with the way the autoload functions are implemented. I know some frameworks do funky dynamic registration on the Composer autoloader to handle plugin code. Allowing them to do the same with a classmap seems beneficial, to the extent that an internal class map is beneficial. Even if it cannot be added to per se, "fetch, modify, and overwrite" seems like a necessary feature, especially if there is a function to retrieve the current map. What other purpose would that function have? 3) The lower-casing of class names feels weird to me. Why? Given that most filesystems are case sensitive, that seems like an odd requirement and the RFC doesn't include an explanation of why that's beneficial. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
> On 16 Mar 2021, at 20:07, Jordi Boggiano wrote: > > Hey, > > Here some perspective on this from the Composer side of things. To the best > of my knowledge the Composer autoloader is the default autoloader used by > most PHP projects at this point except for WordPress. I am sure there are a > few more outliers and people using custom autoloaders in addition of, or > instead of the Composer-generated one, but generally speaking it has > supplanted many custom implementations in frameworks over the years. > > First of all big +1 here, if we can make things faster for non-preloaded > workloads it's great. The autoloader is definitely a big fixed > "infrastructure cost" in many such apps currently, and I don't think we can > squeeze any more perf from userland. > > On 15/03/2021 19:32, Mark Randall wrote: >> On 15/03/2021 17:59, Levi Morrison via internals wrote: >>> IMO, these should be the defined case of the class, or we should be >>> insensitive about it. It's one thing that our symbols are case >>> insensitive; it is wholly another to _require_ it for this feature by >>> requiring lowercase names. I assume there is some motivation for this; >>> I would like to hear it. >> >> EG(class_table) is stored lowercase, the case of the class name itself is >> only known once it has been found which could only occur after the >> autoloader has run. >> >> By forcing lowercase we have a single key to lookup for any variation, >> without it we would have to either match the case exactly (which would be >> different from how internals currently works) or would have to iterate over >> every item in the classmap array each time, performing case a insensitive >> comparisons on every key until one was found, or in the worst case, the >> entire set. > > - The Composer ClassLoader [1] uses a case-sensitive classmap which overall > has not caused major issues except when people do class_exists or new calls > with the wrong case. That said, if we were to support this new API we would > for sure be able to generate a lowercased classmap, we just haven't done it > until now because spl_register_autoload calls the callback with the original > case, and having to lowercase the passed class name on every call seemed > wasteful. If internals already does this anyway that sounds fine by me. > > - It also uses `include` vs the proposed require_once in this RFC. I am not > entirely sure what the implications are here, nor if there are still > performance differences to one over the other, but IIRC we do include because > it's the fastest, and as a class never gets autoloaded twice a file does not > really risk being included again. > > - Right now we have to run includes through a proxy function [2] to avoid > granting the included file access to $this and other in-scope variables. It's > not that much overhead but can quickly lead to a thousand extra function > calls within a request's lifecycle, so just for completeness I figured I'd > mention it. > > - As Mike Schinkel mentioned, the proposed API should really support multiple > maps. Composer a prepend-autoloader option which controls whether the > generated autoloader will be prepended or appended when spl_autoload_register > is called. This is required in some cases where multiple autoloaders coexist. > So the ability to add multiple maps, including optionally prepending them > would be a must IMO. The ability to remove an autoloader and its associated > classmap would also be very welcome so one can unregister things cleanly. > > So taking the above in consideration, and keeping consistency with existing > spl_* functions in mind, I would propose an API more like: > >autoload_register_classmap(array $map, bool $prepend = false) >autoload_unregister_classmap(array $map) > > > P.S: While I am here looking at spl_* docs, it seems to me like > spl_autoload_call should be deprecated in favor of class_exists, and > spl_autoload_extensions + spl_autoload also probably should be deprecated or > at least marked as antiquated in the docs, and not migrated to autoload_* > like you suggested doing for spl_autoload_register/unregister in another RFC. > > [1] > https://github.com/composer/composer/blob/b6826f352390b4c952be8fd75d60cfd4f6f39f11/src/Composer/Autoload/ClassLoader.php > > [2] > https://github.com/composer/composer/blob/b6826f352390b4c952be8fd75d60cfd4f6f39f11/src/Composer/Autoload/ClassLoader.php#L478-L481 > > -- > Jordi Boggiano > @seldaek - https://seld.be > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > Edit: wrong sending address, again. Sending for list, apologies for the dupe. I don’t have a huge amount of interest in this, except to say: If someone wants to add a core function to make Composer’s autoloader work faster, that’s fine and dandy, it doesn’t hurt to provide the building blocks for common patterns. But pushing your
Re: [PHP-DEV] [RFC] Autoloader Classmap
Hey, Here some perspective on this from the Composer side of things. To the best of my knowledge the Composer autoloader is the default autoloader used by most PHP projects at this point except for WordPress. I am sure there are a few more outliers and people using custom autoloaders in addition of, or instead of the Composer-generated one, but generally speaking it has supplanted many custom implementations in frameworks over the years. First of all big +1 here, if we can make things faster for non-preloaded workloads it's great. The autoloader is definitely a big fixed "infrastructure cost" in many such apps currently, and I don't think we can squeeze any more perf from userland. On 15/03/2021 19:32, Mark Randall wrote: On 15/03/2021 17:59, Levi Morrison via internals wrote: IMO, these should be the defined case of the class, or we should be insensitive about it. It's one thing that our symbols are case insensitive; it is wholly another to _require_ it for this feature by requiring lowercase names. I assume there is some motivation for this; I would like to hear it. EG(class_table) is stored lowercase, the case of the class name itself is only known once it has been found which could only occur after the autoloader has run. By forcing lowercase we have a single key to lookup for any variation, without it we would have to either match the case exactly (which would be different from how internals currently works) or would have to iterate over every item in the classmap array each time, performing case a insensitive comparisons on every key until one was found, or in the worst case, the entire set. - The Composer ClassLoader [1] uses a case-sensitive classmap which overall has not caused major issues except when people do class_exists or new calls with the wrong case. That said, if we were to support this new API we would for sure be able to generate a lowercased classmap, we just haven't done it until now because spl_register_autoload calls the callback with the original case, and having to lowercase the passed class name on every call seemed wasteful. If internals already does this anyway that sounds fine by me. - It also uses `include` vs the proposed require_once in this RFC. I am not entirely sure what the implications are here, nor if there are still performance differences to one over the other, but IIRC we do include because it's the fastest, and as a class never gets autoloaded twice a file does not really risk being included again. - Right now we have to run includes through a proxy function [2] to avoid granting the included file access to $this and other in-scope variables. It's not that much overhead but can quickly lead to a thousand extra function calls within a request's lifecycle, so just for completeness I figured I'd mention it. - As Mike Schinkel mentioned, the proposed API should really support multiple maps. Composer a prepend-autoloader option which controls whether the generated autoloader will be prepended or appended when spl_autoload_register is called. This is required in some cases where multiple autoloaders coexist. So the ability to add multiple maps, including optionally prepending them would be a must IMO. The ability to remove an autoloader and its associated classmap would also be very welcome so one can unregister things cleanly. So taking the above in consideration, and keeping consistency with existing spl_* functions in mind, I would propose an API more like: autoload_register_classmap(array $map, bool $prepend = false) autoload_unregister_classmap(array $map) P.S: While I am here looking at spl_* docs, it seems to me like spl_autoload_call should be deprecated in favor of class_exists, and spl_autoload_extensions + spl_autoload also probably should be deprecated or at least marked as antiquated in the docs, and not migrated to autoload_* like you suggested doing for spl_autoload_register/unregister in another RFC. [1] https://github.com/composer/composer/blob/b6826f352390b4c952be8fd75d60cfd4f6f39f11/src/Composer/Autoload/ClassLoader.php [2] https://github.com/composer/composer/blob/b6826f352390b4c952be8fd75d60cfd4f6f39f11/src/Composer/Autoload/ClassLoader.php#L478-L481 -- Jordi Boggiano @seldaek - https://seld.be -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On 15/03/2021 22:18, Levi Morrison via internals wrote: Like any other case insensitive symbol PHP works with that comes from userland, I would lower it in the engine's side of things. I would not push the lowercase requirement on the API. That was the first design, however it required re-creating the array with potentially many thousands of new strings which slowed it down considerably. As I envisenge this to be used with machine generated data, having it use lowercase would not be an issue. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
> On Mar 15, 2021, at 1:41 PM, Mark Randall wrote:
>
> Hi Internals,
>
> I would like to propose the addition of a new mechanism of autoloading
> classes - a classmap that will be consulted prior to checking the
> spl_autoload_register'd callbacks.
>
> https://wiki.php.net/rfc/autoload_classmap
Yes, please! This would be a real treat for those whose use-cases do not fit
nicely into the PSR-4 standard (like WordPress plugins.)
However, may I discuss some additions?
---
1. Sometimes it is inconvenient to set the entire class-map. For example, a
WordPress plugin might want to add all of its files to the autoloader and let
other plugins handle their own needs.
While that could be done with your get() and set() functions along with an
array merge, it would be clearer in userland just to have an add().
Further, when there are tens of plugins then adding the files for each plugin
could be a lot of userland array manipulation that I assume could be done more
efficiently within a core function.
So, I would propose including a third function that would add to the current
classmap rather than just replace it like set():
autoload_add_classmap(mapping $array): void {}
---
2. There has been numerous times when I wanted to monitor when classes are
loaded. I have been reduced to calling get_declared_classes() before
require_once() and then call get_declared_classes() again after and slice off
the difference between the two, which I assume is very costly.
If would be great is the autoloader could call a callable and pass the list of
classes that were loaded, assuming such as callable exists.
So, I would propose including a third function that would add to the current
classmap rather than just replace it like set():
autoload_set_classmap_listener(callable $listener): void {}
autoload_get_classmap_listener(): callable {}
This might be tangential to your RFC but if we are updating the autoloader
already this addition might be trivial.
---
3. Based upon the above and the existing spl_*() functions I would ask it it
would not make sense to create an interface called "Loader" and a set of
classes to support autoloading instead, and then deprecate the spl_*() at some
distant point in the future?
To provide something to make it easier to consider this approach I created a
strawman Gist using PHP code to illustrate the idea. It is not a complete
working example, and likely not fully fleshed out either, but it should work as
a starting point for discussion:
https://gist.github.com/mikeschinkel/2ff8b5f78e88688dbbadb41a762c119f
Note that using an interface like this would allow developers to create their
own custom autoloaders as classes and/or we could implement a PSR-4 specific
autoloader class in the future directly in core that could work with this
configuration (and maybe even a PSR-1 autoloader in core.)
-Mike
P.S. The interface and classes example uses the controversial "PHP" namespace,
but please let us not fixate on that. Let us instead discuss if an interface
and set of classes makes sense and if so figure out the right namespace
afterwards.
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Tue, Mar 16, 2021 at 2:01 AM Christian Schneider wrote: > If the number of classes to load is indeed that big that the function call > overhead is significant enough (I agree with Levi that some numbers > illustrating what we're talking about would be nice), wouldn't that be a > case for class preloading to really speed things up? Preloading doesn't work in every situation, for example Wikipedia depends on one class corresponding to multiple files for progressive deployments; the right file will be determined at runtime. This is not something the preloader can do, AFAIU. For people who are unable to take advantage of I was actually thinking of proposing adding a PSR-4 autoloader to PHP core. -- Best regards, Max Semenik
Re: [PHP-DEV] [RFC] Autoloader Classmap
Am 15.03.2021 um 18:41 schrieb Mark Randall : > I would like to propose the addition of a new mechanism of autoloading > classes - a classmap that will be consulted prior to checking the > spl_autoload_register'd callbacks. > > https://wiki.php.net/rfc/autoload_classmap Just so I understand correctly: This is all about performance, right? Because an autoloader providing this functionality is basically a one-liner, correct? If the number of classes to load is indeed that big that the function call overhead is significant enough (I agree with Levi that some numbers illustrating what we're talking about would be nice), wouldn't that be a case for class preloading to really speed things up? My first impression is that it's a band-aid for overly big class hierarchies without going all the way. Again, maybe some benchmarks could shed some light why this needs to be included in the core. Or maybe I'm missing something. - Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Mon, Mar 15, 2021 at 12:32 PM Mark Randall wrote: > > On 15/03/2021 17:59, Levi Morrison via internals wrote: > > IMO, these should be the defined case of the class, or we should be > > insensitive about it. It's one thing that our symbols are case > > insensitive; it is wholly another to _require_ it for this feature by > > requiring lowercase names. I assume there is some motivation for this; > > I would like to hear it. > > EG(class_table) is stored lowercase, the case of the class name itself > is only known once it has been found which could only occur after the > autoloader has run. > > By forcing lowercase we have a single key to lookup for any variation, > without it we would have to either match the case exactly (which would > be different from how internals currently works) or would have to > iterate over every item in the classmap array each time, performing case > a insensitive comparisons on every key until one was found, or in the > worst case, the entire set. Like any other case insensitive symbol PHP works with that comes from userland, I would lower it in the engine's side of things. I would not push the lowercase requirement on the API. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On 15/03/2021 17:59, Levi Morrison via internals wrote: IMO, these should be the defined case of the class, or we should be insensitive about it. It's one thing that our symbols are case insensitive; it is wholly another to _require_ it for this feature by requiring lowercase names. I assume there is some motivation for this; I would like to hear it. EG(class_table) is stored lowercase, the case of the class name itself is only known once it has been found which could only occur after the autoloader has run. By forcing lowercase we have a single key to lookup for any variation, without it we would have to either match the case exactly (which would be different from how internals currently works) or would have to iterate over every item in the classmap array each time, performing case a insensitive comparisons on every key until one was found, or in the worst case, the entire set. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Autoloader Classmap
On Mon, Mar 15, 2021 at 11:41 AM Mark Randall wrote: > > Hi Internals, > > I would like to propose the addition of a new mechanism of autoloading > classes - a classmap that will be consulted prior to checking the > spl_autoload_register'd callbacks. > > https://wiki.php.net/rfc/autoload_classmap > > Mark Randall > marand...@php.net > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > Overall I am slightly in favor of this. I would like some performance numbers for this, as performance is a considerable motivating consideration. There is one piece that I disagree with, for now at least: > The associative array must contain lowercase class names as the keys, and the > location on disk for the value. IMO, these should be the defined case of the class, or we should be insensitive about it. It's one thing that our symbols are case insensitive; it is wholly another to _require_ it for this feature by requiring lowercase names. I assume there is some motivation for this; I would like to hear it. Again, I'm overall slightly supportive. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] [RFC] Autoloader Classmap
Hi Internals, I would like to propose the addition of a new mechanism of autoloading classes - a classmap that will be consulted prior to checking the spl_autoload_register'd callbacks. https://wiki.php.net/rfc/autoload_classmap Mark Randall marand...@php.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
