Problem/Motivation

We currently include autoload.php twice, once in index.php and again in drupal_classloader() to get the actual $loader instance. The former is only really included so Settings can be initialised in drupal_settings_initialize(), which is called from _drupal_bootstrap_configuration() (and in index.php if an exception is caught). So, if E.g. the APC classloader is used, we are still going to the filesystem to load the settings class via the standard autoloader.

Proposed resolution

manually include the Settings.php class to circumvent using the classloader before drupal_classloader() has had a chance to do its thing.

Remaining tasks

Reviews on approach. Maybe better issue name?

User interface changes

None

API changes

None

CommentFileSizeAuthor
#14 2253593-early-classmap-13.patch3.91 KBneclimdul
#13 2253593-early-classmap-13.interdiff.txt4.5 KBneclimdul
#13 2253593-early-classmap-13.interdiff.txt4.5 KBneclimdul
#10 d8-autoload-preload-2253593-10.patch8.15 KBneclimdul
#7 d8-autoload-preload-2253593.patch6.72 KBneclimdul
d8.autoload-once.patch1.32 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Looks good to me.

Status: Needs review » Needs work

The last submitted patch, d8.autoload-once.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Mehh, Crypt::hmacBase64() is called from drupal_valid_test_ua() which is called from conf_path() :(

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Related issues: +#2023325: Add interface for classloader so it can be cleanly swapped, +#1241190: Possible approaches to bootstrap/front loading of classes, +#1818628: Use Composer's optimized ClassLoader for Core/Component classes
neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Priority: Normal » Major

bumping as a follow up to #2016629: Refactor bootstrap to better utilize the kernel which makes this more important by adding more classes and interfaces to this code path.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Sounds like a good idea to me. Also definitely makes sense to wait on that for sure.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
FileSize
d8-autoload-preload-2253593.patch6.72 KB

Patch built on top of the kernel patch. Won't apply so not bumping status but just as an idea of how we can use composer to do this.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: +Performance
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Related issues: +#2421479: Use a bootstrap.php file to load the most used classes in one go

Added a related issue: #2421479: Use a bootstrap.php file to load the most used classes in one go which kind has a similar idea, but just more radical.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Needs work » Needs review
FileSize
d8-autoload-preload-2253593-10.patch8.15 KB

Here's an updated patch for this issue.

Status: Needs review » Needs work

The last submitted patch, 10: d8-autoload-preload-2253593-10.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

If we have bootstrap.inc included by autoload.php, we have this loaded in unit tests too. Is that what we want?

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Needs work » Needs review
FileSize
2253593-early-classmap-13.interdiff.txt4.5 KB
2253593-early-classmap-13.interdiff.txt4.5 KB

Probably not, that was in the original patch but I think at the very least its outside the scope of this issue.

Also, re-apply accidentally applied composer changes to both composer files but should really have only applied to core/composer.json.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
FileSize
2253593-early-classmap-13.patch3.91 KB

wow... how did I not notice there was no patch here. still had it laying around. still applies.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC, coming from #1818628: Use Composer's optimized ClassLoader for Core/Component classes, I just benchmarked this saves around 0.752 ms, so almost 1 ms, which of 5 ms getting to settings.php and 10 ms for PageCache is a lot.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed eae2db5 on 8.0.x 
    Issue #2253593 by neclimdul, damiankloip: Stop classloader searching...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

This is a problem apparently when you are using composer to build your project, see #2468499: Add vendor libs to classmap through composer script event. Please review the patch over there.