Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.This is a follow-on from #1599108: Allow modules to register services and subscriber services (events), in particular one of Crell's remarks in comment #70 in that issue, i.e.
This may be OK for now, but I wonder if after we have more things in place it would be better to ship a "maintenance mode" DIC precompiled.
.
I had actually added this to the patch but sun expressed some concerns about putting the config services into a precompiled container. For now I'm just attaching the change I had made and then ripped out as a patch here...
This is of course blocked on the other patch.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 1703266.precompiled-dic.patch | 7.71 KB | katbailey |
| precompiled_bootstrap_dic.patch | 5.8 KB | katbailey |
Comments
Comment #1
katbailey commentedI think we are tying ourselves up in knots in certain other patches (e.g. #1702080: Introduce canonical FileStorage + (regular) cache and #1764474: Make Cache interface and backends use the DIC) due to the fact that the config services are defined in one place but used by two different containers - the bootstrap container and the main container.
If we use a precompiled DIC for the bootstrap container and get rid of the idea of merging it into the main container, we then have the freedom to let those two sets of definitions diverge according to what is needed in the two different scenarios.
Comment #2
katbailey commentedComment #3
chx commentedI have two problems with this patch. One, right now you can just do drupal_container()->register('config.factory', ... in settings.php to do an early override. To make sure this already extremely useful feature (and it will get better as we go and move more into the DIC) is kept, the name of the class should not be hardwired. Two, both for doing such an override and for just plain core development, we need a script that generates the precompiled class. Editing the compiled version would be brittle and totally unnecessary. This can't be too hard because I can see (and got comfirtaion on IRC) you generated this with the Symfony PhpDumper. We just need to figure where to store the original code and add a phpdumper call to it, right?
Comment #4
chx commentedClarification: don't do $container = new BootstrapContainer(); but do $container = new variable_get('bootstrap_container', 'Drupal\Core\DependencyInjection\BootstrapContainer');
Comment #5
sunInterestingly, this is not really what I objected to in the original bundles patch -- wasn't there some different code in it?
The proposed changes here are looking actually very similar (or even identical) to the ones I made in #1730834: Remove language() indirection/static cache, consistently use 'language' (factory) service on container
That said, I think we actually want and need to replace the entire hard-coded services here with a "compiled" container in a services.yml file:
i.e., next to settings.php.
That's the only way to properly resolve the race conditions we have with drupal_settings_initialize(), drupal_container(), and $GLOBALS.
Identically to settings.php, the system should simply require and assume for the services.yml file to exist. The installer is the only exception, which can set up a custom ContainerBuilder in install_begin_request(), which is required anyway (and, distros + profiles should be able to enhance/override that one).
Unless I'm mistaken, it's possible that this container can be used as Container (vs. ContainerBuilder) then, which should help with performance.
Comment #6
Crell commentedAren't we trying to kill this? And refiling...
Comment #7
katbailey commentedWell, the ExtensionHandler patch gets us to the point where the minimal container is not used on normal page requests. But to get rid of it completely we'd also need to eliminate scenarios where a container is needed but there is no kernel, either by removing the need for a container in that scenario or by introducing a kernel.
Comment #8
katbailey commentedI am happy to say this issue is no longer relevant :-)