About a year ago, we committed the patch in #658772: Provide way to disable rebuild during features_flush_caches() to add a variable "features_rebuild_on_flush" that could be used to bypass the rebuilding of features whenever cache is cleared. This option is great for speeding up some sites, but has the following bad side-effects:

1) Installing a Module or App that includes a Features Override does not automatically apply the override to Features that are currently in their default state, forcing a manual "features revert"

2) Install profiles need an additional feature_revert_all before a site can be used

3) Changing the code to a Feature and then clearing cache does not properly rebuild the feature.

There might be some other cases that don't work. But most of the time it *does* work and it saves a significant amount of time during cache clear. On a typical Open Atrium site that heavily uses Features and Features overrides, the rebuild can more than double the cache clear time (15 seconds extra on my dev machine).

*Most* of the time, nothing has changed on the site. Poormanscron runs periodically and flushes the cache, causing this extra feature rebuild delay just to determine that nothing needed to be rebuilt. So it would be nice to enable this "features_rebuild_on_flush" variable if the above issues can be addressed.

I'll submit a patch that at least addresses issues 1 & 2 above shortly.

CommentFileSizeAuthor
#1 features_improve_features-2378343-1.patch1.39 KBmpotter
PASSED: [[SimpleTest]]: [MySQL] 40 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

mpotter’s picture

FileSize
1.39 KB
PASSED: [[SimpleTest]]: [MySQL] 40 pass(es). View

Here is the patch for addressing (1) and (2) above. It uses a variable to determine if a module has been enabled or installed since the last time cache was cleared. If the "features_rebuild_on_flush" variable is FALSE and this new "features_modules_changed" variable is set, then the features_rebuild it still forced.

So now, most of the time clearing cache isn't rebuilding the feature. Only after a module has been enabled/installed do features get rebuilt.

Testing this in Open Atrium, it fixes issue (2) for the installer, and also works with (1) when enabling Apps that contain overrides.

I don't see any way to handle case (3) since there is no way to detect that the code has changed other than calling features_rebuild which already does this test which is taking a lot of time. So a dev changing the code would still need a manual features_revert to update the DB with their new code. Which is really the way it's documented anyway.

Currently using/testing this patch in Open Atrium. Comments, suggestions, improvements are welcome!

mpotter’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: features_improve_features-2378343-1.patch, failed testing.

mpotter’s picture

Status: Needs work » Needs review

Try again drupal!

Status: Needs review » Needs work

The last submitted patch, 1: features_improve_features-2378343-1.patch, failed testing.

mpotter’s picture

Status: Needs work » Needs review

ok, come on, I don't think this is failing because of this patch. Something is wrong with the drupal testing infrastructure. Let's try again.

Status: Needs review » Needs work

The last submitted patch, 1: features_improve_features-2378343-1.patch, failed testing.

mpotter’s picture

Looks like testbot is having issues: https://www.drupal.org/node/2375345

Status: Needs work » Needs review
mpotter’s picture

Status: Needs review » Reviewed & tested by the community

Should get this committed finally.

hefox’s picture

Currently it's not checking if the new module is a feature before setting the variable, should it? (based on quickly reading the codE)

mpotter’s picture

@hefox I think this is ok. It's possible to enable a module that implements its own default or alter hooks but isn't technically a "feature". This patch errs on the safe side of rebuilding features if any module is installed/enabled.

mpotter’s picture

Status: Reviewed & tested by the community » Fixed

  • mpotter committed 253d24f on 7.x-2.x
    Issue #2378343 by mpotter: Improve Features performance during cache...

Status: Fixed » Closed (fixed)

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