Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113976
MIME-Version: 1.0
References: <php.internals-113545@news.php.net> <606b8994.1c69fb81.dd253.1b68SMTPIN_ADDED_MISSING@mx.google.com>
In-Reply-To: <606b8994.1c69fb81.dd253.1b68SMTPIN_ADDED_MISSING@mx.google.com>
Date: Tue, 6 Apr 2021 11:01:33 +0300
Message-ID: <CAAwdEzCTB=GON8v2F+sQh=9=x12913y7o2vS1_dJ=fPVxk3fUA@mail.gmail.com>
To: Mark Randall <marandall@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000f5c1e805bf493942"
Subject: Re: [PHP-DEV] Re: [RFC] Autoloader Classmap
From: drealecs@gmail.com (=?UTF-8?Q?Alexandru_P=C4=83tr=C4=83nescu?=)

--000000000000f5c1e805bf493942
Content-Type: text/plain; charset="UTF-8"

On Tue, Apr 6, 2021 at 1:05 AM Mark Randall <marandall@php.net> 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.
>


   - It's not clear from the RFC if the `spl_autoload_call` will use the
   classmap or not. I assume yes but maybe it's good to mention that.
   - There is a mention about another "independent RFC" but I can't seem to
   find any draft of it or other reference. Maybe add some more links and
   clarify when it is expected to be filled and by who.
   - It was previously discussed but it's still not clear why the key
   should be lowercase.
      - As far as I understand, when the class autoloading is triggered,
      the classname is available with it's requested case so that is
not an issue.
      - It is inconsistent with how `autoload_register` callable works.
      - I would like to have things like
      autoload_set_classmap([ClassToBeLoaded::class =>
      '/specific/path/for/class.php]) without adding an extra `strtolower`
      wrapping call.
      - Is there a performance benefit of having all classes with lowercase?
      - If a class autoloading is triggered with the wrong case (due to a
      typing error), I would very much like to fallback to classic autoloaders
      and fail so that I can possibly find the issue faster.
   - Related to"Trigger a TypeError if the key's associated value is not a
   string.", shouldn't we validate the value when setting up the function.
   Having a TypeError when autoloading happens signaling an invalid
   configuration sounds a bit weird.
   - Does the extra step to check the classmap also increase time if the
   class is not found? If most of the classes are loaded using classic
   autoloaders, will there be an impact from having some classes defined for
   the classmap autoloader?
   - How was the test that showed the 5% run? Was there any IO involved
   during the classloading? How was the opcache configured? There are some
   times mentioned there but it's not clear the unit of measure.
   - I understood that in real life the 5% is only out of the 0.1 - 1% that
   is classloading out of a script's runtime. That would mean an 0.005 to
   0.05% performance increase. It's unclear to me how much of a script's time
   the autoloading takes. Maybe you can clear that out if you counted for a
   normal application run, like a hello world from symfony. I understand that
   there will be nothing visible but maybe it's good to mention that, just to
   have clear expectations.
   - Maybe you can give a sample code for when the autoload_set_classmap
   needs to be used from two different contexts/modules that are not aware
   about the others. I would expect something like:
      - `autoload_set_classmap(autoload_get_classmap() + $moduleClassMap)`,
      not overriding previously set map values, if duplicates

Regards,
Alex

--000000000000f5c1e805bf493942--