Re: function autoloading v4 RFC

On Mon, Aug 19, 2024, at 14:03, Gina P. Banyard wrote: > On Thursday, 15 August 2024 at 17:22, Rob Landers <rob@bottled.codes> wrote: >> Hello internals, >> >> I've decided to attempt an RFC for function autoloading. After reading hundreds of >> ancient (and recent) emails relating to the topic along with several abandoned RFCs from the past, >> and after much review, I've decided to put forth a variation of a previous RFC, as it seemed >> the least ambitious and the most likely to work: >> >> https://wiki.php.net/rfc/function_autoloading4 >> >> Please let me know what you think. An implementation should be along before opening it for >> a vote (now that I realize how important that is). > > I had a quick glance at the RFC, and I really don't think this is a good approach or good > API. > Having autoloading in SPL causes dumb issues, for example ext/session must register it has a > dependency on ext/spl just for the autoloader. [1] > This means that currently ext/session depends on ext/spl, ext/spl depends on ext/standard, and > ext/standard depends on ext/session, a glorious circular dependency. > > This main problem has pushed me again to rebase my PR [2] for the Core Autoloading RFC. [3] > The wording of the RFC hasn't been updated, as this is frankly my least favourite part of > any RFC. > The PR passes CI and has also produced benchmark results from CI. [4] > > I would rather have some collaboration on a proper solution than, IMHO, this suboptimal > solution. > I still need to get opinions from other people if it makes sense to remove the > zend_autoload_class function pointer API and have the VM directly call zend_autoload. > Because from what I remember 2 years ago, some profiling tools hook into it to track > autoloading time. > This might be improved by introducing new observer hooks. > > > Best regards, > > Gina P. Banyard >> > > [1] > https://github.com/php/php-src/pull/14544#issuecomment-2294907817 > [2] https://github.com/php/php-src/pull/8294 > [3] https://wiki.php.net/rfc/core-autoloading > [4] https://github.com/php/php-src/actions/runs/10441267948 > Hey Gina, I’d love to collaborate on this feature. For what it’s worth though, I did a ton of research on it (mostly reading every discussion I could find on the topic, and prior RFCs) and I felt that this API was the most likely to be accepted. You even have a comment on your PR asking why not an API similar to this one (!) though your reasoning is sound for why it is a bad idea, and I believe it is the superior API. There are some key differences between our two RFC’s that I think are worth discussing (besides the obvious API differences): 1. in your RFC, it calls the autoloader with the global function it isn’t found in both scopes. In mine, it calls it once not found in the local scope and calls the autoloader once (for the local scope). This seemed to be a highly liked proposal in one of the older discussions (2013-ish?) that appeared to not result in a new RFC, as it would bypass a lot of perceived performance issues in earlier RFCs. If an autoloader so desires, the autoloader can check the global scope (by getting the “base name” of the function), but autoloading the global scope should be a niche application these days. 2. I like the “pinning” aspect. I haven’t seen your code yet, but I suspect it just registers the global function in the current namespace? If so, does this affect the __namespace__ global? Probably not, but I am just curious. What happens if I manually require a file with a pinned function in it? 3. Should we update function_exists like I did in mine to include an autoload argument? 4. You mention no default autoloader for classes and functions, while I agree that this should be the case, will the spl library still provide the default class autoloader that can be registered? As far as performance for ambiguous functions go, I was thinking of submitting an RFC, where ambiguous function calls are tagged during compilation and always resolve lexically, sorta like how it works now: echo strlen($x); // resolves to global, always require_once "my-strlen"; echo strlen($x); // resolves to my strlen, always This works by basically rewriting the function name once resolved and may make the code more predictable. If I can pull it off, it can be relegated to a technical change that doesn’t need an RFC. Still working on it. In other words, maybe pinning could be solved more generally in a future RFC, decrease your RFC’s scope and chance for sharp edge cases. In any case, if you are up for a high bandwidth conversation to collaborate, we can join a call, collaborate in the open, or whatever you feel most comfortable with. I’m very excited to see this feature. — Rob