CRM-13796 Don't CRM_Core_Invoke::rebuildMenuAndCaches(TRUE) on every Drupal module enable / disable

This might happen on Drupal cache clear as well?

Agree that it would be a performance boost to skip cache-resets when the Civi hooks aren't involved. (As an aside, there are a few hooks that pose the same issue with cache-resets and extension-lifecycle – h_c_triggerInfo, h_c_xmlMenu, h_c_managed off the top of my head.)

The great thing about these hooks is that they're relatively simple and robust – e.g. the underlying DB records are managed correctly when installing, when editing code, when mucking about (as a zealous newbie DBA), when upgrading, when uninstalling. And it works the same with all CMS's. (Note, for example, that one can use Drupal's module_implements() to "identify CiviCRM-related modules" but Joomla has no analog.) We would want to preserve these properties.

I suggest approaching this optimization in a slightly different way:

a) In the case of triggers, the work involves a few things: (1) calling h_c_triggerInfo to build a list of triggers and (2) updating the DB to use the triggers. Part #2 is much more expensive than #1. My suggestion would be keep part #1 as-is but skip part #2 when it's appears unneeded. For example, you might change CRM_Core_DAO::triggerRebuild to something like https://gist.github.com/totten/9fc4ebfc2c8bce3b031d (which is rough - note the FIXME's).

b) Similarly, for h_c_managed, CRM_Core_ManagedEntities::reconcile() could compute a signature for $this->declarations and use that to decide if it needs to run the full reconciliation logic.

I ran into this again this afternoon, and foolishly didn't look back here at your comments before implementing a crude check in the Drupal module to avoid calling the rebuild.

I've submitted that PR anyway, but keen to discuss - it looks like your suggested approach would improve things for a wider use case than just Drupal module changes.

The list of hooks this checks for is pretty arbitrary

Quick reactions:

1. Yeah, I definitely prefer the gist's approach. Main reason: it provides the same benefit for Drupal modules, Civi extensions, and all other CMS plugins. Secondary benefit: it avoids other unnecessary trigger-rebuilds. (Ex: The heuristic in #414 would rebuild triggers if a module implements hook_civicrm_permission, even though that doesn't involve triggers.)

2. Agree that #414 would mitigate the issue on Drupal without major breakage.

3. IIRC, the extension-lifecycle hooks (e.g. hook_civicrm_install) are only supported by native extensions. The CMS's already provide their own lifecycle hooks (e.g. hook_install in Drupal). The extension-lifecycle hooks don't need to be checked here.

4. Theoretically, #414 could introduce a subtle regression if you have an extension which defines new (non-core) hooks. For example, extension A (CiviVolunteer, CiviRules, etc) might define hook_civicrm_widgets and store that in a cache. Drupal module B implements hook_civicrm_widgets. During installation, you need to flush the widget cache... but there's no way for civicrm-drupal.git to know about this cache apriori. But is this theory or reality?

• There are definitely some extensions which define new hooks (e.g. civibuild create universe and grep lre '>invoke' --exclude-dir=vendor | grep -v ^civicrm). I tend to assume/take-for-granted that it's useful to link hooks with caches, but it would take a lot of reading to identify whether this is actually done in any of the matching extensions.
• If someone does implement hook_civicrm_widgets, then they probably also implement one of the other Civi hooks (e.g. hook_civicrm_config). That might keep the issue asymptomatic, but it's not particularly reliable engineering.

I've closed the PR based on Tim's feedback.
Chris Burgess do you want to take another crack at it?