Change default for CIVICRM_CONTAINER_CACHE to simplify admin/developer experience
authorTim Otten <totten@civicrm.org>
Thu, 5 Jul 2018 21:12:39 +0000 (14:12 -0700)
committerTim Otten <totten@civicrm.org>
Thu, 5 Jul 2018 22:38:32 +0000 (15:38 -0700)
In discussion of cache-related PRs for Civi 5.3, there were a few
reports/issues from developers observing `ServiceNotFoundException`.  This
is because there's not much awareness about how service-definitions are
cached.  It shouldn't be a significant issue for production systems running
vanilla code, but for admins and developers (who juggle patches/branches),
it can cause confusion/support-issues/false-failures.  This PR aims to
reduce those.

(This is a follow-up/substitute for #12401.)

Before
------
* The default value of `CIVICRM_CONTAINER_CACHE` is `always`.

After
-----
* The default value of `CIVICRM_CONTAINER_CACHE` is `auto`.

Technical Details
-----------------
The Symfony container essentially stores a list of "services".  In some
Symfony-based apps, building the list of services can be time consuming, so
it's common to cache this.

In Civi, this cache physically appears as
`files/civicrm/templates_c/CachedCiviContainer.*.php`.  The constant
`CIVICRM_CONTAINER_CACHE` determines how Civi manages the cache.  There are
three available policies:

* `never`: This means we never use the cache.  You never have to worry about
  staleness.  But it means we have to fire `hook_civicrm_container` on every
  page-request, and we go through any container-compilation tasks.
  This would have the fewest support-issues for devs and advanced admins.

* `always`: This means we always use whatever is in the cache.  It never
  (automatically) rebuilds the cache or checks freshness...  if you make
  change, then you must flush the cache explicitly.  This should be the
  fastest in production.

* `auto`: This means we typically use the cache (avoiding
  hooks/recompilation), but it has to `stat()` a half-dozen files to check
  the modification time.  (To wit: if the timestamp on
  `Civi/Core/Container.php` changes, then we discard the cache.)

Since performance is a consideration, I did some light benchmarking on my
laptop (fetching a basic public Civi page 100 times across 3 threads).

https://gist.github.com/totten/ec4bffd723afb7967aec56a3040b9ca3

In these results, the `never` policy appears to be ~15-20ms slower than
`auto` or `always`. `auto` is only ~2ms slower than `always`.

The other consideration is accuracy -- `auto` will usually re-compile if you
make a change, but there are some edge-cases where you must still flush
manually.  (In particular -- when you first implement
`hook_civicrm_container` in a new extension, it might not be aware of the
new extension.  And extensions need to call `$container->addResource()`.)

However, overall, `auto` is a pretty good compromise that's almost as fast
you can get and works out-of-the-box for many dev/admin scenarios.

Civi/Core/Container.php

index ffe1bfbe3bcc93284bd68df4ecb706e19f5de7f5..17d0be551993f971f21c50ec56a94cf77f118299 100644 (file)
@@ -65,7 +65,7 @@ class Container {
     // services. Consequently, we assume a minimal service available -- the classloader
     // has been setup, and civicrm.settings.php is loaded, but nothing else works.
 
-    $cacheMode = defined('CIVICRM_CONTAINER_CACHE') ? CIVICRM_CONTAINER_CACHE : 'always';
+    $cacheMode = defined('CIVICRM_CONTAINER_CACHE') ? CIVICRM_CONTAINER_CACHE : 'auto';
 
     // In pre-installation environments, don't bother with caching.
     if (!defined('CIVICRM_TEMPLATE_COMPILEDIR') || !defined('CIVICRM_DSN') || $cacheMode === 'never' || \CRM_Utils_System::isInUpgradeMode()) {