Api4 Services - Lazy-load subscriber-objects

[totten]

This refines the way in which several subscribers (Civi/Api4/Event/Subscriber/**.php) are loaded. This makes it safer (from a performance POV) to continue adding more subscribers/listeners without worrying that it will impact the quantity of files/classes/opcodes/SLOC loaded in a typical page-view. To wit: subscribers are only loaded if they are needed.

A good way to visualize the mechanics of this change is to skim getDispatcherService() ([civicrm.compile]/CachedCiviContainer.*.php) before and after the patch. (Examples included below.)

Before

During every page-load, you need to register event-listeners. For subscriber-objects (like Civi/Api4/Event/Subscriber/**.php), you get the list of subscriptions by calling getSubscribedEvents(). Therefore, on every page-load, you must load/process the subscriber and calll getSubscribedEvents() (regardless of whether the subscriber will actually be used -- on the chance it that may be needed).

In CachedCiviContainer.*.php, you will see snippets like:

    protected function getDispatcherService()
    {
        ...
        $instance->addSubscriber(${($_ = isset($this->services['Civi_Api4_Event_Subscriber_ActivityPreCreationSubscriber']) ? $this->services['Civi_Api4_Event_Subscriber_ActivityPreCreationSubscriber'] : ($this->services['Civi_Api4_Event_Subscriber_ActivityPreCreationSubscriber'] = new \Civi\Api4\Event\Subscriber\ActivityPreCreationSubscriber())) && false ?: '_'});
        $instance->addSubscriber(${($_ = isset($this->services['Civi_Api4_Event_Subscriber_ActivitySchemaMapSubscriber']) ? $this->services['Civi_Api4_Event_Subscriber_ActivitySchemaMapSubscriber'] : ($this->services['Civi_Api4_Event_Subscriber_ActivitySchemaMapSubscriber'] = new \Civi\Api4\Event\Subscriber\ActivitySchemaMapSubscriber())) && false ?: '_'});
        ...

Observe that it instantiates ActivitySchemaMapSubscriber then passes the instance to addSubscriber().

After

You only need to instantiate service-objects if (a) you are building a fresh container or (b) you are actually running a relevant event.

This works by calling getSubscribedEvents() when building the container. The list of events is cached in the container.

In CachedCiviContainer.*.php, you will see snippets like:

    protected function getDispatcherService()
    {
        ...
        $instance->addSubscriberServiceMap('Civi_Api4_Event_Subscriber_ActivityPreCreationSubscriber', ['civi.api.prepare' => 'onApiPrepare']);
        $instance->addSubscriberServiceMap('Civi_Api4_Event_Subscriber_ActivitySchemaMapSubscriber', ['api.schema_map.build' => 'onSchemaBuild']);
        ...

Observe that it alludes to ActivityPreCreationSubscriber symbolicly but does not need an actual instance.

Comments

1. To see that this is equivalent, I used cv debug:event-dispatcher before and after the patch. This requires an updated version of cv, and the formatting is a little a different, but it does show the same list of listeners.

2. There could be some concern like, "What happens if you're upgrading and have a cached list of subscription events?" Well, note that CRM_Api4_Services::hook_container already puts a cached list of subscribers in the container. It already registers the FileResource(s). Thus, if a file Civi/Api4/Event/Subscriber/**.php changes, it already makes the decision to recompile based on filemtime().

3. There should already be a lot of test-coverage which hits code-paths for these listeners. (If this area is generally non-functional, you should see massive failures.)

4. For r-run, I picked an arbitrary subscriber (eg ActivitySchemaMapSubscriber.php), then:

   • At the start of the file, add a statement to log whenever the file is read.

     file_put_contents('/tmp/parselog.txt', sprintf("%s: %s: %s\n\n",date('Y-m-d H:i:s'), __FILE__, \CRM_Core_Error::formatBacktrace(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 15))), FILE_APPEND);

   • In a separate window, run tail -f /tmp/parselog.txt.
   • Edit the chosen subscriber (eg ActivitySchemaMapSubscriber.php) to add/remove listeners (like hook_civicrm_alterContent)
   • Request some Civi page (curl 'http://dmaster.127.0.0.1.nip.io:8001/civicrm/admin?reset=1'). It's not important that it actually runs the full page... just that we boot up Civi to look for the page.
   • Alternately repeat the past few steps. Observe thta it only parses the file if there has been a change or if the relevant event(s) actually fire.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

+1 on the code & concept.
This would have massive test failures if it didn't work.

[seamuslee001]

Agree with Coleman's thoughts on this one