Core PHP autoloader proposal

[aristath]

Using a PHP autoloader can improve performance, and can be seen as a first step towards modernizing the WP codebase in the future.

This ticket is a continuation of the discussion on #36335
The discussion in the original ticket was derailed many times, so this fresh ticket is an opportunity to discuss the Core proposal submitted on the ​Make blog

The concerns that were brought-up in the original ticket:

• PHP 5.2 compatibility: No longer relevant.
• Files containing classes also contain other functions: This has already been addressed since then. The few instances where this was still an issue were addressed in the patch/PR.
• Composer: Outside the scope of this initial implementation.
• Overriding Core classes from plugins: Added a site-health check with a critical warning in case a Core class is not loaded from the default WP path.
• API for plugins/themes to register additional classes: Outside the scope of this PR.
• WordPress is not OOP: True, it's not currently OOP, but as we integrate the new editor in WP-Core, more and more OOP concepts creep in Core. There is definitely a trend, and WP is slowly becoming more OOP than it used to be. An autoloader will unblock us and allow us to further optimize and improve Core as we go.
• What does autoloading do for "loose" functions?: Nothing. But it does give us the flexibility to properly group them in classes in the future, and only load what's needed. But that is a future discussion that will probably take a few more years.
• Backwards-compatibility issues: There are none.
• Performance: This patch has a negligible positive effect on performance. That is not its point. It opens the door to future improvements that can't be done easily without an autoloader. This is not an improvement with an immediately tangible outcome, it's just paving the way for more meaningful future improvements.

Trac ticket: https://core.trac.wordpress.org/ticket/60414

[justlevine]

Love this!

@aristath can you clarify how (if at all) this relates to the ​Autoload class used by Requests])? You mentioned in the Proposal that an API is outside of the scope, and I'm assuming that since Requests is also intended to be consumed outside of WP contexts that it'l continue to maintain it's own implementation, and I'm not seeing anything directly related in the diff,

(Don't want to derail this by another decade by bringing up Composer again - so for people coming here directly and not ​from the Make post, let's hold on to this:

This is the first step towards modernizing the WP codebase. Though in itself it may appear like a marginal (but still very worthy) performance improvement, it opens the door to more improvements in the future of WordPress without any backwards-compatibility concerns.

and ​this followup comment)

[aristath]

Replying to justlevine:

@aristath can you clarify how (if at all) this relates to the ​Autoload class used by Requests])? You mentioned in the Proposal that an API is outside of the scope, and I'm assuming that since Requests is also intended to be consumed outside of WP contexts that it'l continue to maintain it's own implementation, and I'm not seeing anything directly related in the diff,

Check out the WP_Autoload::register_external_bundled() method.
It registers the autoloaders from external libraries inside the Core autoloader. Hard includes/requires for these were then removed from the rest of the WP codebase, and the Core autoloader handles them 👍

Adding some numbers from tests ran today.

## Included files

Adding this snippet in mu-plugins/snippets.php:

<?php
add_action( 'shutdown', function() {
        var_dump( count( get_included_files() ) );
} );

I got the following numbers:
| | Trunk | Autoload | Diff

Front	442	347	-95 files (-21.5%)
Dashboard	479	370	-109 files (-22.7%)

I only checked the frontpage and the main dashboard page, so these numbers will differ depending on the context. Some pages will have smaller differences, others will have larger. I expect that REST-API endpoints will have a significantly larger benefit, but I don't know exactly how to measure that.

## Memory usage.

Tested using the ​Debug Bar community plugin. Numbers in Kb
| | Trunk | Autoload | Diff

Front	4327	4329	0.04%
Dashboard	4074	4078	0.09%

This shows that the autoloader implementation currently uses 0.04-0.09% more memory. I don't know if that is because I have XDebug running (which admittedly is a performance hog), these numbers contradict previous tests both by @spacedmonkey in ​https://github.com/WordPress/wordpress-develop/pull/3470#issuecomment-1490083546 as well as previous tests of mine. Worth examining what changed these past few months.

## Time

Tested using XDebug profiling. Report built using webgrind.
These numbers are the average of 5 runs because XDebug-profiling produces rather inconsistent numbers.

	Trunk	Autoload	Diff
Front	392ms	377ms	-3.82%
Dashboard	317ms	295ms	-6.94%

---

The above tests are somewhat basic.
If someone else has the ability and/or ability to run some more detailed tests to get more consistent numbers, it would be greatly appreciated 👍

I have played a little with benchmarking, doing 100 sequential requests to each endpoint. The results should be more consistent, but I'm not sure what is the reason for quite large variance (look at the standard derivation value for timing).

Benchmarking was running against PHP 8.1 with nginx server. Profiling data comes from xhprof (slightly better than xDebug and provides actual memory usage info).

### Average Run Times (mean) / (median) / (stddev)

	trunk	autoload	Percentage Diff
admin	125.79ms / 121.37ms / ±22.38	125.02ms / 121.66ms / ±17.82	-0.62%
front	228.13ms / 223.95ms / ±13.45	230.23ms / 224.68ms / ±16.14	0.92%
rest	91.55ms / 89.32ms / ±13.23	87.20ms / 86.25ms / ±5.17	-4.75%

### Average Memory Usage (mean) / (median) / (stddev)

	trunk	autoload	Percentage Diff
admin	6.04M / 6.04M / ±0.04	6.23M / 6.23M / ±0.00	3.08%
front	6.66M / 6.66M / ±0.00	6.69M / 6.69M / ±0.00	0.49%
rest	5.68M / 5.68M / ±0.00	5.71M / 5.71M / ±0.00	0.5%

I've left notes on how my benchmark was conducted, along with collected results in repository: ​https://github.com/bart-jaskulski/wordpress-benchmark

Since I lack the ability to test memory consumption reliably, I think a good measure would be the test-suite results posted in this PR.

​https://github.com/WordPress/wordpress-develop/actions/runs/8264293168

• There seems to be virtually no difference in the memory before/after this PR.
• The differences in load-times is ±1ms in almost all areas when ran in the test-suite.

Have you considered having multiple smaller autoload.php files instead of just one big src/wp-includes/class-wp-autoload.php ? This way each module will have its own autoload map. Those issues with simple-pie and parser.php having multiple classes in them will be addressed this way, with a very local approach. In theory, having smaller autoloads will also help to test each module in isolation without relying on the global core autoload.

I'm curious if you have explored bundling ​Composer\Autoload\ClassLoader instead of building a new autoloader? It's more or less the standard nowadays, it's battle-tested, and it already supports working with classmaps

$composer = new \Composer\Autoload\ClassLoader();

 $composer->addClassMap([
        'wp_hook'    => 'wp-includes/class-wp-hook.php',
        'wp_locale'  => 'wp-includes/class-wp-locale.php',
]);

$composer->register();

It seems to me there will be an additional benefit as there are a lot of plugins and themes that are already using composer, although in a local capacity.

I'm curious if you have explored bundling ​Composer\Autoload\ClassLoader instead of building a new autoloader? It's more or less the standard nowadays, it's battle-tested, and it already supports working with classmaps

Acutely aware, discussion of it completely derailed the last attempt to include composer setting things back years. Having said that there was discussion of using composer to generate the file mappings, but you're best referring to the Make WP post for specifics.

I'm not suggesting using composer as it is used in general, instead only utilizing the ClassLoader as available solution. I did manage to read the previous thread, and you are right - the composer discussion did seem to get out of hand.

[Kaloyan]

There are places in the code with defensive class_exists() checks before class declarations, like this:

if ( ! class_exists( 'Some_Class', false ) ) class Some_Class {}

Considering the autoloading changes, do you think those defensive checks are still going to be necessary?

[aristath]

@Kaloyan I just did a quick search in WP's code for class_exists checks.
It looks like the only remaining instances are in 3rd-party scripts, and one more instance in deprecated.php (which is safe to ignore) 👍

Added an additional check in site-health.

If there are any overriden classes, it will show a critical security warning.

To test this, I added a file in mu-plugins/w.php with the following contents:

<?php
class WP_Community_Events {}
class WP_Users_List_Table extends WP_List_Table {}

We can fine-tune the text, presentation etc, but I believe this can serve us well and inform users if any Core classes are being overriden by a plugin/theme/host.

[johnbillion]

This was briefly discussed during today's performance team ticket scrub.

• What is needed to get this approved and ready for a merge into core?
• Any outstanding concerns from the comments on the make/core post or from the PR?
• This is likely now too late for 6.6, so let's target 6.7.
• This needs a core committer to own it at the beginning of the 6.7 cycle.

[aristath]

A quick update of what happens in this ticket and a recap of the concerns that were brought up in the proposal - along with what was done to address them:

• Composer vs non-composer: It's easier for us to start with the basic classmap implementation that exists in the PR, and then discuss separately if we want to automate the classmap generation using Composer.
• Added an extra check in the health-check screen, to check if any Core classes are being overridden, and show a critical warning if something like that occurs
• PHPUnit tests were added and all existing automated tests pass without issues.
• Performance impact: Adding an autoloader seems to reduce the memory use, but - at this stage - has no other significantly measurable impact.

Any original concerns that were mentioned in the original ticket (#36335) have already been added to this ticket's description, along with what was done to address them

The PR is complete and it works. There are no backward-compatibility issues, and I constantly update it so it's up to date with trunk.

[dd32]

I was asked privately to test this on WordPress.org, I don't foresee this causing any issue (but always worth testing..) but I do have a request, that a heads up be given to meta to ensure it doesn't cause a short w.org downtime.

It might also be advisable to make it in two commits, 1) Addition of the autoloader & wp-settings require of it, and 2) the actual removals of the manual inclusions.

That would only really be of benefit to WordPress.org due to how we deploy code, but if given a heads up that it's in a single commit I can probably manually deploy the autoloader ahead of time to avoid issues.

In briefly testing it on WordPress.org, the only potential issues I've run into are detailed in ​https://github.com/WordPress/wordpress-develop/pull/3470#pullrequestreview-2209084648

[hellofromTonya]

Sharing a summary of a Making WordPress ​core-committers Slack channel discussion:

@spacedmonkey asked:

What is the process of getting a ticket as big as #60414 committed? What is needed to make the core committer feel confidence in the change? Would a change like this need two or more committers to sign it off?

@costdev shared:

Good question. While I don't have a specific number of committers in mind, I'd say at least 3-4 committers should have reviewed/tested this since it's sort of a big change (in additon to other reviewers/testers). Maybe also looking to WP-CLI and similar to ensure no unexpected impacts are felt.

@hellofromTonya shared:

In addition, IMO Core Tech Lead(s) (for the targeted major release) should approve or at least give the okay for it to be included in the major.

For this kind of big refactoring change, approval should happen as early as possible in the alpha cycle. Doing so will give plenty of time for feedback and follow-ups.

@jorbin shared:

If you look at the [38571] refactoring of hooks as a similiar sized risk, that had 8 committers propped. I'm not sure that exact number is needed, but it does show there was a wide amount of testing.

The associated ticket #17817 also shows a couple of things that I think will help:

1. Getting the patch on a dotorg sandbox and making sure it doesn't blow up .org.
2. Getting this on a large host so we can see how it performs in the myriad conditions core runs
3. Making sure that wp-cli doesn't break

I think 3-4 committers that are willing to usher it through the release as the likelihood of there being breakage is decent and having more folks very familiar is super helpful.

I also think there are unresolved concerns on ​the make/core post that make me think we aren't at consensus on if this is a good idea.

This PR expands on amazing work in ​https://github.com/WordPress/wordpress-develop/pull/3470 by @aristath.

This uses composer auto generated class_map to autoload files.

To test locally, checkout code and run, composer install, this will generate composer autoloader locally.

Trac ticket: https://core.trac.wordpress.org/ticket/60414

[spacedmonkey]

So we can compare the options, I have worked on ​PR to use the autoloader generated by composer. In my testing, it seems work well.

Can you take a look @aristath and see if you can compare it against your solution. Why was composer rejected in the first place?

[aristath]

Why was composer rejected in the first place?

Composer was not "rejected". However, the whole Composer debate derailed the conversation multiple times, and delayed the effort to add an autoloader in Core for years.

Let's first get a basic implementation merged before we re-ignite that debate.

I tried to explain the reasoning in the ​proposal on make.w.org:

Since the Composer vs non-composer autoloader was one of the main points of discussion in the 8-year-old ticket, it’s worth mentioning here, and clarify some points:

Composer is a very mature and safe way to autoload classes. However, this initial implementation does not use it.

This proposal is for a minimally invasive, performance-optimized change. It implements autoloading using a hardcoded classMap. Still, It does take care of all the preliminary work so if it is decided that a Composer autoloader is what we want to do, then switching to that will be a relatively easy task. The current PR has a very narrow focus and serves as the first step towards modernizing the WP codebase.

Composer would allow us to automate generating the autoloader, so if deemed appropriate, we could modify the current proposal to implement that.

Adding an initial, minimal autoloader will not block a future Composer implementation. It is easier to start without it, and then add it if we find that we need it. Doing the reverse could prove more difficult, so this path was deemed safer as a first step.

[jorbin]

I think any autoloader needs to make it so that files can not be overloaded and if that's not possible, this ticket should be closed as wontfix. Allowing extenders to override core files means:

1) Core can't rely on functionality existing from one version to another in a different file. This will cause sites to break on upgrade.

2) Since a core file can be overridden once, it makes it harder for multiple plugins to extend the same functionality.

3) Instead of encouraging collaboration and making WordPress better, plugins will just override core files to fix a bug they are facing since it will be easier for them.

While the intent isn't to allow plugins to override files, the fact that this is a possibility makes the idea of an autoloader one that won't benefit the end users of WordPress.

[justlevine]

Replying to jorbin:

I think any autoloader needs to make it so that files can not be overloaded and if that's not possible, this ticket should be closed as wontfix.

Idk that feels pretty arbitrary to me. While I definitely want to see overloading as well (and as soon as possible), even this "barebones" approach that means you no longer need to litter your code with conditional require_once calls is already a significant DX win and a way to make concrete progress on an 8-year old issue.

[azaozz]

Replying to justlevine:

While I definitely want to see overloading as well (and as soon as possible)

Seems the term "overloading" may be misunderstood? This is not the OOP overloading of methods in a particular class. Here overloading refers to overriding, i.e. replacing the file where a class is defined. That would replace portion of the WordPress code with some arbitrary code that cannot be maintained simultaneously with the rest of the core code.

A basic example of a "disaster waiting to happen" situation is when a plugin replaces the definition of a core class, and in the next WP release that class is updated and contains few new methods. Assuming that the plugin is also updated, if there is any misalignment between the core update and the plugin update, the sites with that misalignment will be likely to throw fatal errors. If this was done in a "popular" plugin, there may be millions of sites throwing fatal errors because of that.

What if the plugin was not updated?

I'm with @jorbin here. Unfortunately unless overriding/replacement of core files is made impossible this would be better as "wontfix".

[spacedmonkey]

I think we maybe able to solve the overriding class issue. I know @aristath was looking into this. Please do not close this ticket until all these options have been explored.

Also something to consider, instead of autoloading all core classes, why not load classes for a sub system like rest api, widgets or customizor.

In the case of rest api and widgets, core widgets / endpoints can already be unregistered and replaced with your own class. Overloading these classes would not be new functionality in these cases.

[justlevine]

Replying to azaozz:

Replying to justlevine:

While I definitely want to see overloading as well (and as soon as possible)

Seems the term "overloading" may be misunderstood? This is not the OOP overloading of methods in a particular class.

Nope, was typing from my phone and just missed a word, should have been 'want to see overloading prevented'. Will update my comment.

Years of __experimental Gutenberg methods released in core, even though people technically could and IRL did (ab)use them in their own plugin/themes. Iteratively theyre being locked down or removed.

Obviously if there's a way to block autoloading from the getgo we should, but marking wontfix instead of addressing that in a followup-ticket just to prevent devs fron possibly doing stupid things feels harsh 🤷 just my 2 cents.

[peterwilsoncc]

I figured I'd check the practicality of overriding a core class. Regrettably it's exceptionally practical as is.

<?php
namespace No\No\No\No\No;

spl_autoload_register( __NAMESPACE__ . '\\wp_post_replace', true, true );

function wp_post_replace( $class_name ) {
        $class_name = strtolower( $class_name );
        if ( 'wp_post' !== $class_name ) {
                return;
        }
        require_once __DIR__ . '/overrides/class-wp-post.php';
}

Like @spacedmonkey, I think it would be good to wait for Ari's investigation in to seeing if it is possible to prevent it.

Along similar lines, it's also regrettably practical to override a WordPress JavaScript file as Core provides a filter for the purpose:

<?php
namespace No\No\No\No\No;

add_filter( 'script_loader_src', __NAMESPACE__ . '\\wp_data_replace', 10, 2 );

function wp_data_replace( $src, $handle ) {
        if ( 'wp-data' === $handle ) {
                $src = content_url( '/mu-plugins/overrides/wp-data.js' );
        }

        return $src;
}

The code differs but it's something WordPress contributors do in the Gutenberg plugin.

As contributors, it's generally considered that a plugin replacing WordPress scripts is to run WordPress outside of the warranty. Could replacing classes be considered in the same way and would it be possible for the plugin review team to block plugins replacing core classes?

[aristath]

Are we saying that we don't want to do something in WordPress Core, because there's a possibility a plugin might do something bad? Plugins already have access to the filesystem and the database. They can already do whatever they want. They can even overwrite Core code and classes if they want - as demonstrated in a previous ​comment in the make post.

Thankfully, we have rigorous plugin reviews that guard the community against such things and malicious behaviors.
It would be relatively easy to add an automated check to the plugin-checker, and simplify checking for plugins overriding a Core class.

We can't prevent bad things, but we can warn users if we detect a plugin doing something we consider bad. With that in mind, this implementation/PR/patch includes a new check to the health screen, throwing a critical security issue in case a plugin does something like overriding a Core class.

1) Core can't rely on functionality existing from one version to another in a different file. This will cause sites to break on upgrade.
2) Since a core file can be overridden once, it makes it harder for multiple plugins to extend the same functionality.

If a plugin does something bad, it can break a site. OK... How is that different from what we have now? To quote a ​comment from @joostdevalk, "That’s like not going from petrol to electric cars because you can still have an accident with an electric car."
Plugins breaking on Core update, or breaking each other, is nothing new. It's the de-facto reality of any open system like WordPress. As long as people build plugins, there will always be a chance they'll do something wrong.
We can limit the possibility by not allowing plugins on .org to override Core classes.
We can guard users by throwing a critical security warning when we detect such behavior so they are aware of the danger of the plugin they're using.
What we can't do, is change the reality that if someone wants to do something bad with WordPress, they'll do it - especially since they already have access to the filesystem & db.

3) Instead of encouraging collaboration and making WordPress better, plugins will just override core files to fix a bug they are facing since it will be easier for them.

That's not going to happen for plugins hosted on .org. For non-org plugins, there's no way to check what they do, and as I mentioned above, they can already do whatever they want - including overwriting Core classes.
If a developer feels that they can't collaborate or contribute and they resort to releasing a plugin not hosted on .org, then that won't be because we added an autoloader... It will be because we've made contributing and collaborating an unpleasant and hostile experience.

[hellofromTonya]

IMO the way to frame the discussion is to focus on this question:

Should Core introduce a brand new customization mechanism that allows extenders to replace Core files?

Without preventing overriding, this autoloader will add this brand new customization mechanism. Thus, it will by design allow the means to replace Core's native classes. As with any other customization mechanism (such as hooks), if it's available natively in Core, it will be used.

For the WordPress open source project, this mechanism can be beneficial for Gutenberg and feature plugins.

But is it beneficial to the ​majority of users?

These are the questions to answer IMO.

[azaozz]

Replying to justlevine:

Nope, was typing from my phone and just missed a word, should have been 'want to see overloading prevented'.

Ah, I see. Seems we share pretty much the same opinion, thanks for clearing this!

marking wontfix instead of addressing that in a followup-ticket just to prevent devs from possibly doing stupid things feels harsh

Yea, you're right this feels harsh. However decisions like this would make everybody's life easier in the future as WP strives to maintain maximum backwards compatibility. By "everybody" I mean WP contributors, committers, and users.

Frankly I wish there were more "harsh" decisions like that in the past. Would have prevented many security vulnerabilities and made it easier to maintain certain features and components (for example shortcodes).

[azaozz]

Replying to peterwilsoncc:

Along similar lines, it's also regrettably practical to override a WordPress JavaScript file as Core provides a filter for the purpose:

Right, however JavaScript works quite differently than PHP. It is a prototype-based language, i.e. most functions, objects (classes), methods, etc. can be replaced/overridden at any time by any script, even the built-in functions. There isn't such thing as Fatal error: Cannot redeclare function_name() (previously declared in....) in JS.

For that reason it seems the ability to replace core scripts from PHP was intentional in the first iterations of Script Loader back in 2007-2008.

[azaozz]

Replying to aristath:

Are we saying that we don't want to do something in WordPress Core, because there's a possibility a plugin might do something bad?

As far as I see, not quite. The difference here is to impose certain restrictions on what plugins can do. This is similar to adding a private method to a class, instead of adding a public method and hope that plugins won't use it.

I agree it is a harsher way to do this (see previous comment), however imho this would be a good "code design" decision that would make everybody's life easier in the future.

[peterwilsoncc]

But is it beneficial to the majority of users?

These are the questions to answer IMO.

Coming back to @hellofromTonya's question, I've been considering where this could be beneficial beyond developers:

1. Performance: if it is shown that the performance of the front-end of a site improves with an autoloader then it becomes a benefit for all users.

2. Increased likelihood of catching bugs in Gutenberg and other feature plugins.

While the Core (and, I imagine, theme and plugin teams) wouldn't want developers overridding core classes, it could prove beneficial for features developed via a plugin.

In Gutenberg and other feature plugins, core classes are often extended and essentially replaced during the development process. For example ​Gutenberg_REST_Templates_Controller_6_6 extends WP_REST_Templates_Controller and subsequently replaces the endpoint.

This often complicates the merge process and increases the risk that a bug will be introduced during that process or an order of operations issue is discovered post merge.

If feature plugins can override the core classes, then I think it's a benefit to users when it comes to the stability of code eventually merged in to WordPress-Develop.