Problem/Motivation

SystemRequirementsHooks::checkRequirements() resets the module + theme extension lists.

Because it's called on every admin page due to update requirements, this means that cache_bootstrap is invalidated every request when you're navigating around admin pages.

We can add a way to get the uncached extension list without invalidating the cached one.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3588490-nr-bot_ekuvd8y3.txt1.15 KBneeds-review-queue-bot

Issue fork drupal-3588490

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

catch opened merge request !15668

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3588490-nr-bot_ekuvd8y3.txt1.15 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Priority: Normal » Major

how come this happened?! sounds major at least

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs change record
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Nice find, I commented on why this IMHO fails so hard right now. There's also #3336621: Running hook_requirements() on all admin pages is very expensive to help with the "runs on every/many admin pages" and improve that.

catch’s picture

catch
he/him
Primary language English
commented

@andypost I think this goes back to Drupal 7 or earlier and has been ported around.

catch’s picture

catch
he/him
Primary language English
commented

Drupal 7 system_requirements() calls system_rebuild_module_data() - which both built the system data but also wrote it, so that confirms 'ported from at least Drupal 7' for why it's like this.

https://api.drupal.org/api/drupal/modules%21system%21system.install/func...
https://api.drupal.org/api/drupal/modules%21system%21system.module/funct...

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
catch’s picture

catch
he/him
Primary language English
commented
Issue tags: -Needs change record

Added a change record.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Page loading performance

Thank you

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#3336621: Running hook_requirements() on all admin pages is very expensive
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Nice find. Can add typehinting on the new params.

nicxvan’s picture

nicxvan commented
Component: system.module » extension system
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

Addressed the reviews on the MR I think.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Feedback from me and alexpott was addressed, I think this can go back to RTBC.

godotislate made their first commit to this issue’s fork.

godotislate closed merge request !15668

  • godotislate committed 7918d127 on main 
    task: #3588490 Don't reset the extension lists in system requirements...

  • godotislate committed 8f55502d on 11.x 
    task: #3588490 Don't reset the extension lists in system requirements...

  • godotislate committed 4985bc5a on 11.4.x 
    task: #3588490 Don't reset the extension lists in system requirements...
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7918d12 main, 8f55502 to 11.x, and 4985bc5 to 11.4.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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