Revealed via #910156: Remove 'description' keys from schema cache, we're loading the entire 'schema' cache on every page, in case there is an entity loaded for anything - including menu links.

CommentFileSizeAuthor
#11 drupal.entity-schema-cache.11.patch2.87 KBsun
#7 drupal.entity-schema-cache.7.patch3.81 KBsun
#5 drupal.entity-schema-cache.5.patch2.18 KBsun
#3 drupal.entity-schema-cache.3.patch2.11 KBsun
#2 drupal.entity-schema-cache.2.patch2.3 KBsun
#1 drupal.entity-schema-cache.1.patch2.29 KBsun
drupal.entity-schema-cache.0.patch1.83 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
FileSize
drupal.entity-schema-cache.1.patch2.29 KB

Even better!

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
FileSize
drupal.entity-schema-cache.2.patch2.3 KB

oopsie

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Priority: Normal » Major
FileSize
drupal.entity-schema-cache.3.patch2.11 KB

Status: Needs review » Needs work

The last submitted patch, drupal.entity-schema-cache.3.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal.entity-schema-cache.5.patch2.18 KB

Remote entities do not specify a 'base table'.

Status: Needs review » Needs work

The last submitted patch, drupal.entity-schema-cache.5.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal.entity-schema-cache.7.patch3.81 KB

Weird tests introduced in #742972: Clear entity info cache during module enable

chx’s picture

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

I have generated nodes and terms and then the front page showed a nice 4.5% performance increase. That's a steal given how simple the patch is. Awesome work.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Very, very nice.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Reviewed & tested by the community » Needs work

No, we don't want to change the test case like that, because it was explicitly written to check for hook_watchdog(). As I described in #742972: Clear entity info cache during module enable hook_watchdog() should be consistent and developers should be able to rely on the fact that a hook in their module is only called when it is fully enabled and installed.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
drupal.entity-schema-cache.11.patch2.87 KB

Let's fix those tests properly over there.

The only required fix for this patch is the removal of the non-existing and therefore invalid 'base table' definition.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented

Ähm, why is 'base table' invalid? It is listed on http://api.drupal.org/api/function/hook_entity_info/7

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Because the specified table does not exist.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented

Ah right, that thing isn't generated automatically. Sorry.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Thus, back to RTBC, I guess.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Although my local box is totally not suitable for doing benchmarks, I've mass-applied the following range of current performance patches and am getting a nice boost:

#910190: Entire schema cache needlessly loaded on all pages
#910156: Remove 'description' keys from schema cache
#606966: 'load arguments' of parent path are not inherited
#910124: Clean up menu_contextual_links()

BEFORE patches:
Executed 36 queries in 25.18 ms.
Page execution time was 481.48 ms.
Memory used at: devel_boot()=2.5 MB, devel_shutdown()=16.23 MB, PHP peak=16.75 MB.

AFTER patches:
Executed 35 queries in 15.99 ms.
Page execution time was 471.18 ms.
Memory used at: devel_boot()=1.95 MB, devel_shutdown()=15.85 MB, PHP peak=16.5 MB.

Test page: admin/config/development/devel; Garland; enabled Navigation menu block containing 1 taxonomy term link and 1 node link.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Nice catch!!

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Performance

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

Issue tags: +Performance

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.