That means, in my case where I noticed it, 18 identical cache get queries instead of one.

Patch coming in a second.

CommentFileSizeAuthor
#3 system-list-cache-1969814-3.patch3.78 KBberdir
#3 system-list-cache-1969814-3-test-only.patch3.24 KBberdir
#2 drupal-1969814-2.patch1.83 KBdawehner
#1 system-list-cache-1969814-1.patch545 bytesberdir

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new system-list-cache-1969814-1.patch545 bytes

Here we go. If only every performance bug were as easily solved ;)

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new drupal-1969814-2.patch1.83 KB

The fix looks perfect!

I tried to write a test, though it fails during the "installation", maybe you have an idea

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new system-list-cache-1969814-3-test-only.patch3.24 KB
new system-list-cache-1969814-3.patch3.78 KB

Thank for starting the test. This seems to work, had to add a factory.

Status: Needs review » Needs work

The last submitted patch, system-list-cache-1969814-3-test-only.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
commented

Thanks for making the test work!

+++ b/core/lib/Drupal/Core/Cache/MemoryCounterBackendFactory.phpundefined
@@ -0,0 +1,35 @@
+   * @param $bin

needs strongs.

+++ b/core/lib/Drupal/Core/Cache/MemoryCounterBackendFactory.phpundefined
@@ -0,0 +1,35 @@
+  function get($bin) {

should be public

chx’s picture

chx commented

I am hesitant on this one. Can we throw system_list out instead at least for modules? It is in the container, after all.

chx’s picture

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

I guess it's a go and #7 is a followup?

catch’s picture

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

Why is this being called 12 times?

In Drupal 7 only the bootstrap list has static caching (because in Drupal 6 the bootstrap list is actually queried twice - once for boot once for exit iirc), but unless something changed recently the bug doesn't exist in D7 and if anything 8.x should be calling it less rather than more.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

The code in 7.x and 8.x of that function looks quite different, see http://api.drupal.org/api/drupal/includes%21module.inc/function/system_l...

In 7.x, it differentiated between called with type bootstrap, calling it multiple times with that argument would also lead to this bug and the "elseif (!isset($lists['module_enabled'])) {", which would then get the information from the cache.

I could do something similar and wrap the whole thing into an if and return at the end but that results in a bigger patch than this :) I'm fine with changing it...

catch’s picture

catch
he/him
Primary language English
commented

Well it's more wondering why it's getting called so much at all - there's only about three places that call system_list() and those at least used to have their own static caches.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Issue tags: -Performance

#3: system-list-cache-1969814-3.patch queued for re-testing.

alansaviolobo queued 3: system-list-cache-1969814-3.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 3: system-list-cache-1969814-3.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue summary: View changes
Status: Needs work » Closed (outdated)
Issue tags: +Bug Smash Initiative

This seems long obsolete, system_list() is gone and replaced with ModuleHandler and ThemeHandler, both of these cache their lists internally and do not regenerate them on every list call.