On sites with lots of exportable entities (e.g. rules) rebuilding entities takes a seriously long time (up to 60s!!).

We'll need to make importing defaults more efficient.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fago’s picture

Status: Active » Needs review
FileSize
4.83 KB

ok, attached patch should optimize it. In my tests this already helped a lot, however another big problem are multiple menu_rebuilds caused by modules upon entity-hooks.

Modules can easily skip that menu_rebuild on cache-clear by checking for the is_rebuild property as in the patches attached for rules_link and profile2, however that way a double-cache clear what be required for the chances to take affect as menu_rebuild() runs before we can rebuild entities.. :/

As a workaround, we could just issue another menu_rebuild() in the entity api centrally. That would be already a lot better than having 10 menu rebuilds caused by 10 rules links, whereas a menu_rebuild took about 4s on my test-site of 7s complete cache clear time (including rebuilding entities with numerous rules).

Attached patch overhauls entity-rebuilding logic to be more efficient, by doing a single multiple entity load. It also fixes #1361588: deleted exported entities don't disappear - including tests.

fago’s picture

rules link and profile2 menu_rebuild skip patches attached.

Status: Needs review » Needs work

The last submitted patch, profile2_menu_skip.patch, failed testing.

fago’s picture

Status: Needs work » Needs review
FileSize
992 bytes
642 bytes

ok, we should not directly call menu_rebuild(). Still, we need to move entity_defaults_rebuild() before menu_rebuild() on cache clear.

Status: Needs review » Needs work

The last submitted patch, profile2_menu_skip.patch, failed testing.

fago’s picture

Status: Needs work » Needs review
FileSize
6.95 KB

ok, here is a variant of the entity patch that actually solves it by doing the magic in hook_menu().
Not ideal as it is going to run on every module rebuild, but probably the best we can do until with a separate rebuild step for that in core.

mh86’s picture

FileSize
989 bytes

updated profile2 patch as it didn't apply with git and drush make

Status: Needs review » Needs work

The last submitted patch, profile2_menu_skip_1.patch, failed testing.

fago’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, profile2_menu_skip_1.patch, failed testing.

fago’s picture

Status: Needs work » Fixed
FileSize
4.83 KB

ok, based on the experiences from #1404198-18: Separate database cache clearing from static cache clearing and data structure rebuilding I don't think we should move rebuilding exportables to such an early stage, but better accept sometimes not 100% correctly refreshed caches (what we cannot guarantee anyway with the current d7 cache-clear approach...).

Anyway, the approach from #6 but kept in hook_flush_caches() is already an improvement. However, a big improvement comes by optimizing menu-rebuild strategies in modules, in particular rules-link and profile2:

Patch for rules-link: #1406296: Improve menu rebuilding logic
Committed fix for profile2: http://drupal.org/commitlog/commit/12182/5c66f8f0cda95911dc85d04a4c283c3...
Committed patch for the entity api attached.

fago’s picture

Also, modules that need to rebuild the menu upon exportable creation/changes/deletion should make sure to make use of the variable_set() approach instead of directly calling menu_rebuild():

+/**
+ * Implements hook_rules_link_insert().
+ */
+function rules_link_rules_link_insert($link) {
+  // Do not directly issue menu rebuilds here to avoid potentially multiple
+  // rebuilds. Instead, let menu_get_item() issue the rebuild on the next page.
+  variable_set('menu_rebuild_needed', TRUE);
+}

Status: Fixed » Closed (fixed)

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