[wp-trac] [WordPress Trac] #36335: Next generation: core autoloader proposal

[WordPress Trac]

#36335: Next generation: core autoloader proposal
-----------------------------+------------------
 Reporter:  dnaber-de        |       Owner:
     Type:  feature request  |      Status:  new
 Priority:  normal           |   Milestone:  4.7
Component:  General          |     Version:
 Severity:  normal           |  Resolution:
 Keywords:  has-patch        |     Focuses:
-----------------------------+------------------

Comment (by MikeSchinkel):

 Replying to [comment:218 TJNowell]:
 > It's not wether autoloader X or autoloader Y is faster. It's wether
 autoloading vs no autoloading is faster.
 > We need to demonstrate with hard facts that an improvement is possible.

 100% agreed.

 > I don't see why we need to rename our files, the part were we find the
 classes and their filenames is meant to happen at buildtime, it's just a
 PHP array `'class' => 'filename'`, why overcomplicate things?


 Great question.  If we use a classmap autoloader there is absolutely no
 reason we would be required to rearrange files. We can get a working
 autoloader without rearranging files.

 The reason I have been thinking it would be good to rearrange files for
 the benefits I will mention next but maybe I jumped the gun with this
 idea. Maybe baby steps are better.

 So I fully admit that my interest in renaming them are based on assumed
 concerns and not valid benchmarks, and I agree we should benchmark it.

 So we can discuss after benchmarking my concern was adding to the memory
 footprint from loading the full classmap.  Loading the classmap is
 probably fine on 90+% of sites, but large traffic sites that can't cache
 everything might be negatively affected by loading a classmap on every
 page.

 How?  We could store a number instead of the actual class paths  where the
 number represents the root for the autoload directories, e.g.:

 - `1` for `ABSPATH . 'wp-admin/autoload/'` and
 - `2` for `ABSPATH . 'wp-includes/autoload/'`.

 From there we could just:

 {{{
 require "{$root_dir}/{$class_name}";
 }}}

 This '''''might''''' make a tangible difference in memory usage for high
 traffic sites, but very admittedly it might not. I am''(/had?)'' planned
 to rework the autoloader and classmap generator so we can benchmark this.
 Should I ''(not)?''

 > I don't understand why we need to rearrange most of core, this sounds
 like a recipe for disaster and failed auto-updates. It screams fragile

 1. I do not understand how this can cause failed updates. I do not mean
 that I debate you on this, I mean I honestly am not aware of how this
 might break things.  Can you elaborate?

 2. One of the strategies I have been thinking about is for us to leave the
 files in place for all the classes we move to an autoload directory. Then
 in those files we change them to all include only one line, a `require()`
 that would call `_deprecated_file()`. That way I think there would be
 almost zero chance that we could break anything, but please check my logic
 on this.

 >  - There may be things we don't want to autoload that are always loaded,
 or too critical to let a plugin override

 And there is no problem here as autoloading is orthogonal to explicit
 loading. Simply leave the hardcoded `require()` in place for those files,
 they will be loaded exactly as before and having those files are in the
 classmap would have zero effect.

 I could even enhance the classmap generator to omit from the classmap any
 classes you explicitly blacklist, e.g.

 {{{
 // wp-classmap-generator.php

 require __DIR__ . '/tools/classmap/class-classmap-generator.php';

 $generator = new WP_Classmap_Generator( __DIR__ . '/src' );
 $generator->add_files( 'wp-admin' );
 $generator->add_files( 'wp-includes' );
 $generator->omit_classes( array(
         'WP',
         'WP_Query',
         'WP_Post',
         'WP_Rewrite',
         'wpdb',
 ));
 $classmap = $generator->get_classmap();
 file_put_contents( __DIR__ . '/src/wp-classmap.php', $classmap );
 }}}

 Matter of fact, I updated
 [https://github.com/staylor/develop.wordpress/pull/2 the PR] to include a
 `omit_classes()` method with the above listed classes omitted.

 > As for benchmarks, install Query Monitor and run a vanilla install with
 and without these changes, and measure 10 common page loads 10 times. That
 should give crude timings. Better yet use a cli tool

 Great suggestion.  I can't promise I will get these done immediately, but
 I will definitely tackle them once my priority client work is out of the
 way.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36335#comment:219>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform