Autoloading WordPress classes

[aristath]

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

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[tomdevisser]

Ohhh, love this PR!

[spacedmonkey]

Run some profiles, before and after, the results, are sadly not promising.

2023 theme

Before

After

2021 theme

Before

After

[aristath]

Run some profiles, before and after, the results, are sadly not promising.

Not quite... 😄
The numbers on the screenshots above point out something interesting:
PHP time is increased by ~1.5%, but memory consumption is decreased by ~4.5%.

Worth noting:
This is just the 1st step towards a much bigger project to modernize WP and make it more efficient, it's not the end of the road.
There are many things we can do to improve and modernize WP, but none of that can happen if WP keeps loading everything, always.
Implementing a simple autoloader doesn't have a big impact in itself (the benefits and costs are marginal, and one could argue that a 4.5% decrease in memory-cost is more important than a 1% increase in time-cost), but it does open the door to many other improvements 👍
The real performance improvements will come after the autoloader has landed, where we'll be able to take advantage of it. This PR just adds the necessary infrastructure for the future 😉

[lkraav]

Implementing a simple autoloader doesn't have a big impact in itself (the benefits and costs are marginal, and one could argue that a 4.5% decrease in memory-cost is more important than a 1% increase in time-cost), but it does open the door to many other improvements +1

I'm also thinking that at very least, it's a DX improvement and future unlocker. If it lands with no performance regressions, that's cherry on top.

[lkraav]

Hey @aristath are you daily driving this by any chance, or is anybody else?

Wondering if I could become real hands-on for testing this in my semi-production / staging env 🤔 or will everything blow up instantly.

[aristath]

Hey @lkraav!

The implementation here is complete, and the autoloader works as expected. I haven't seen anything "blow up"...
Once/week I rebase the PR and update it, pulling the latest changes from Core, updating anything that needs updating, resolving conflicts etc, keeping it up to date with WP trunk.
This definitely needs lots of testing, so if you use it on your semi-production/staging sites that would be great, and it would allow us to further polish the implementation! 🙇

I plan to create a new trac ticket when the time comes and publish a formal proposal, so any testing and additional data we have will be greatly appreciated! 👍

[lkraav]

Once/week I rebase the PR and update it, pulling the latest changes from Core, updating anything that needs updating, resolving conflicts etc, keeping it up to date with WP trunk.

Ah, this brings up a compatibility question - I'd like to stay in at least release candidate stability, but I guess this branch is constantly moving with trunk? Maybe you have tags available so I could merge only up to some "latest compatible with 6.1, or 6.2" state?

[aristath]

I'd like to stay in at least release candidate stability, but I guess this branch is constantly moving with trunk?

Yeah, I'm keeping it up to date with trunk constantly...
However, the patch should apply cleanly to RC!

[lkraav]

I'd like to stay in at least release candidate stability, but I guess this branch is constantly moving with trunk?

Yeah, I'm keeping it up to date with trunk constantly... However, the patch should apply cleanly to RC!

I'm running into trouble trying to apply it to release WP at https://github.com/WordPress/WordPress.git didn't think of that before.

src/ would be easy to strip, but looks like I also need to skip tests 🤔

EDIT git apply 3470.patch --exclude=tests/* --exclude=phpunit/* -p2 --reject 💪

This hunk gets rejected on top of 6.2 RC2

$ [-] cat index.php.rej 
diff a/index.php b/index.php    (rejected hunks)
@@ -19,9 +19,8 @@
 
 /*
  * Load the actual index.php file if the assets were already built.
- * Note: WPINC is not defined yet, it is defined later in wp-settings.php.
  */
-if ( file_exists( ABSPATH . 'wp-includes/js/dist/edit-post.js' ) ) {
+if ( file_exists( ABSPATH . WPINC . '/js/dist/edit-post.js' ) ) {
        require_once ABSPATH . '_index.php';
        return;
 }

Aha but I see this index.php is build-only, so perhaps all good!

Skimmed through our sites, and everything seems to be working 💪 no crashes!

[kasparsd]

@spacedmonkey The front-end performance test suite for this pull request appears to show a performance improvement for both classic and block themes:

That's with 20 runs against the trunk baseline.

[spacedmonkey]

This file will need to be broken up into multiple files.
https://github.com/WordPress/WordPress/blob/10838cb5a009a95de13fa4d2f50afabcb7ca26c7/wp-includes/class-wp-block-parser.php

[aristath]

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.

[kktsvetkov]

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.

[kktsvetkov]

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.

[kktsvetkov]

This is curious -- is src/wp-includes/plugin.php included/required before the wp-autoload is set? And that's why it is necessary to include it here as well?

[aristath]

I don't remember exactly the scenario that required this... But in all cases where I added this line in various files, it was because without it I was getting errors either when using WP, or when running PHPUnit tests 🤔
I don't remember exactly the case, but this is one of the files that plugins/themes frequently include manually in order to load various functions, so maybe that's why I was getting errors 🤷

[tomjn]

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

@kktsvetkov 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, this isn't the place for it

[kktsvetkov]

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.

[aristath]

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.