Problem/Motivation
In most cases a service doesn't need to be instantiated, along with all if its dependencies, in order to determine which hooks it implements. Reflection can be used directly with a hooks class. Discovery should be optimized to not always directly get instantiated services from the container.
Results of discovery can be cached and utilized on production environments. Discovery caches should use fastchained, as these caches will be used for most requests.
Caching should be opt in. Not opting in won't have a significant performance penalty.
Provide documentation and guidance on how to use caches and add a hook_requirements to warn site builders.
Proposed resolution
- Separate hooks discovery + make results serializable.
- Update docs
- Create d.o guide and optimized docs.
User interface changes
New line item always present on Drupal's status page.
API changes
Internal API changes
Data model changes
New serialized cache
New container parameter
Issue fork hux-3276804
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
alexpottI was reading the current code and the runtime use of reflection on every alter and hook invocation scared me from a performance perspective. Great to see this issue already exists!
This is a critical issue if this module is to get widespread usage / inform future choices Drupal core might make.
Comment #4
alexpottReading the issue summary.
Are you sure about this? Doesn't it depend on how many classes and hooks are using it?
Comment #5
dpi@alexpott We're using Hux, alongside many Hooks classes with some decent DI, and to date I'm not seeing significant performance loss while profiling with Blackfire. In fact while testing requests with this patch its often a single-digit millisecond or so slower.
I'm thinking reflection is fast enough, or PHP8 has some kind of optimizations/internal caching around it. In any case I was really hoping to see significant slowdowns without this patch, and inversely gains with this patch, but have not seen real evidence yet.
I hope to be proven wrong!
Comment #7
dpiCommitted!
I'm up for re-evaluating default settings if we find performance is significant without this setting turned on enabled.
I plan to periodically profile performance impact with this off, especially as Hux implementations increase over time.