field_sync_field_status() runs for each module enabled / installed.
It uses system_rebuild_module_data(), which means that the data for all modules needs to be rebuilt (and all of their info files reparsed).
This gets old really fast. On an install profile with 133 modules (Kickstart v2), it adds minutes to the profile install time.

Original issue

The call to system_rebuild_module_data() in field_sync_field_status() is unnecessary and slows down installation noticeably due to the cache rebuilding taking place.

You might see #1599146: field_modules_enabled() greatly slows down profile installs, especially with features modules included and #1574716: Avoid unnecessary cache rebuilds when creating and updating fields for earlier discussion.

Here I am trying out a suggestion by bojanz from those threads, which is to do something like this - new code in the if-block, old code in the else-block for clarity:

<?php
/**
* Refreshes the 'active' and 'storage_active' columns for fields.
*/
function field_sync_field_status() {
 
// Refresh the 'active' and 'storage_active' columns according to the current
  // set of enabled modules.
  //
  // But don't run the full system_rebuild_module_data() during installation,
  // as it is slow.
 
if (variable_get('install_task') != 'done') {
   
$modules = module_list(TRUE);
    foreach (
$modules as $module) {
     
field_associate_fields($module);
    }
  }
  else {
   
$all_modules = system_rebuild_module_data();
   
$modules = array();
    foreach (
$all_modules as $module_name => $module) {
      if (
$module->status) {
       
$modules[] = $module_name;
       
field_associate_fields($module_name);
      }
    }
  }
 
db_update('field_config')
    ->
fields(array('active' => 0))
    ->
condition('module', $modules, 'NOT IN')
    ->
execute();
 
db_update('field_config')
    ->
fields(array('storage_active' => 0))
    ->
condition('storage_module', $modules, 'NOT IN')
    ->
execute();
}
?>

For our large install profile, we found that this was a good speed-up and did not cause any unexpected issues.

Files: 
CommentFileSizeAuthor
#10 1599306-field-sync-status-d7-do-not-test.patch801 bytesbojanz
#8 1599306-field-sync-status.patch821 bytesbojanz
PASSED: [[SimpleTest]]: [MySQL] 37,019 pass(es).
[ View ]
#7 1599306-field-sync-status.patch870 bytesbojanz
PASSED: [[SimpleTest]]: [MySQL] 37,025 pass(es).
[ View ]
#5 1599306-field-sync-status.patch1.79 KBbojanz
FAILED: [[SimpleTest]]: [MySQL] 37,011 pass(es), 36 fail(s), and 2 exception(s).
[ View ]
#1 fields-remove-unnecessary-rebuild-1599306.patch1.2 KBexratione
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in fields-remove-unnecessary-rebuild-1599306.patch.
[ View ]

Comments

StatusFileSize
new1.2 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in fields-remove-unnecessary-rebuild-1599306.patch.
[ View ]

Here is the patch version of that code snippet above.

Status:Active» Needs review

This will most likely need to be rolled against 8.x--setting to needs review to see what the test bot has to say.

Status:Needs review» Needs work

The last submitted patch, fields-remove-unnecessary-rebuild-1599306.patch, failed testing.

Version:7.x-dev» 8.x-dev

This needs to be redone.

For all intended purposes, field_sync_field_status() is just a very stupid and inefficient way of doing a module_list(). At this point of the installation process, the module list has been properly rebuild, so there is absolutely no reason to rebuild it ourselves.

So let's just replace all this by:

<?php
  $modules
= module_list();
  foreach (
$modules as $module_name) {
   
field_associate_fields($module_name);
  }
?>

... and move on with our lives :)

Title:Cache rebuild via system_rebuild_module_data() called in field_sync_field_status() greatly slows down profile installsfield_sync_field_status() needlessly rebuilds module data, slows down installation dramatically.
Status:Needs work» Needs review
StatusFileSize
new1.79 KB
FAILED: [[SimpleTest]]: [MySQL] 37,011 pass(es), 36 fail(s), and 2 exception(s).
[ View ]

Here's a patch that removes field_sync_field_status() completely.
I see no reason why that function should run on cache clear and cron, if it already runs each time a module is enabled and disabled. That covers it.
Maybe yched can point out something that I'm missing.

Alternatively, field_sync_field_status() can be modified to call module_list() instead of system_rebuild_module_data(), which will at least kill the big performance impact (imagine installing 133 modules like Kickstart v2 does. At the end, it reparses 133 info files, and regenerates all data. Ouch!).

Status:Needs review» Needs work

The last submitted patch, 1599306-field-sync-status.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new870 bytes
PASSED: [[SimpleTest]]: [MySQL] 37,025 pass(es).
[ View ]

Okay, let's try and be conservative then. Keeping field_sync_field_status().

StatusFileSize
new821 bytes
PASSED: [[SimpleTest]]: [MySQL] 37,019 pass(es).
[ View ]

Simpler.

Running this on Kickstart v2 reduced the number of calls to system_rebuild_module_data() from 149 to 9, which decreased the total number of function calls during install by 34.9%, and decreased the total CPU time by 30.4%.

Status:Needs review» Reviewed & tested by the community

This makes total sense.

Here's the D7 version for later.

Nice find. Looks good to me.

Issue summary:View changes

Added short summary.

Status:Reviewed & tested by the community» Fixed
Issue tags:+7.15 release notes

Wow, great find!

Committed and pushed to 8.x and 7.x. Thanks! A performance increase like this might make sense to mention in the release notes for 7.15, too.

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

Issue summary:View changes

Clarify summary.