Minor optimization for RecipeRunner::processInstall()

Created on 9 November 2023, 11 months ago
Updated 11 September 2024, 17 days ago

Problem/Motivation

While stepping through the install process, I noticed this:

    foreach ($install->modules as $name) {
      // Disable configuration entity install but use the config directory from
      // the module.
      \Drupal::service('config.installer')->setSyncing(TRUE);
…
      \Drupal::service('config.installer')->setSyncing(FALSE);
    }

(Same for themes.)

Steps to reproduce

N/A

Proposed resolution

Rather than repeating this in each loop, do it only once.

Remaining tasks

User interface changes

API changes

Data model changes

πŸ“Œ Task
Status

Closed: outdated

Version

11.0 πŸ”₯

Component
recipe systemΒ  β†’

Last updated 1 day ago

Created by

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @wim leers
  • @wim-leers opened merge request.
  • Status changed to Needs review 11 months ago
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • Status changed to RTBC 11 months ago
  • πŸ‡§πŸ‡ͺBelgium borisson_ Mechelen, πŸ‡§πŸ‡ͺ

    Seems easy enough and should increase performance. LGTM.

  • Status changed to Needs review 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States thejimbirch Cape Cod, Massachusetts
  • Status changed to RTBC 17 days ago
  • πŸ‡¨πŸ‡¦Canada b_sharpe

    Though in my tests the performance impact is negligible, this makes sense and has no impact on functionality. RTBC

  • Status changed to Closed: outdated 17 days ago
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    There's definitely no real performance benefit here. Also looking at \Drupal\Core\Extension\ModuleInstaller::updateKernel() we already take care of sync status across rebuilds of the container due to module install.

    However this code has been extensively refactored so recipes can be installed in batch operation so the approach is invalid and the MR does not apply to 11.x core or recipes fork.

    In light of the batch refactor I do not think this is worth pursuing.

Production build 0.71.5 2024