Here's the deal:
We don't want to split all module files into includes, because that makes no difference for opcode cache-enabled sites.
However, what makes sense for all sites is:
Not loading dead code.
Dead code?
Plenty.
All hooks in core modules that integrate with another, optional core module. A few things jump into my mind:
- Block
- Search
- Trigger (revert this)
- Actions
- RDF
- Taxonomy
- optionally: Help (not sure about abuse in core without Help module being enabled)
Of course, it only makes sense to move functions into an include, when the implementing module does not depend on the module that exposes the hooks. Example for that: Forum module depends on Taxonomy, so it makes no sense to move taxonomy hooks into a forum.taxonomy.inc.
But for everything else: Dead code. Loaded, cached, but totally unused through the entire lifetime of a site.
Since that code is used by the default profile, benchmarks need to be done with the Expert profile, additionally enabling only some of those optional modules.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | drupal-hooks-hook-info.3.patch | 5.43 KB | sun |
| #10 | registry.php_.3.txt | 6.41 KB | sun |
| #10 | drupal-hooks.patch | 137.19 KB | sun |
| #8 | drupal.hooks-hook-info.2.patch | 8.95 KB | sun |
| #8 | registry.php_.2.txt | 7.12 KB | sun |
Comments
Comment #1
sunoopsie. I wanted to do an initial patch, but let's agree on the targets first.
Also attaching the rewrite.php script I originally wrote for the registry split.
Comment #2
moshe weitzman commentedat first blush, block and help look like a good candidates. not sure the others are common enough to matter.
Comment #3
Crell commentedThat sounds like a good starting point indeed.
However, rarely-used-by-design hooks like hook_menu() and hook_theme() are similarly "dead" on 99% of page requests. APC sites don't care, but non-APC sites do.
So would we then have:
$module.block.inc
$module.search.inc
$module.actions.inc
$module.taxonomy.inc
$module.rdf.inc
$module.build.inc (menu, theme, et al)
I'm not sure I agree with the caveat for, say, forum not splitting out its taxonomy hooks. Even if they're required, not splitting them out means that some core modules will have their hooks in the .module file and some won't. That's inconsistent. If we're going to make dependent module-hooks the standard convention, let's apply that consistently.
Note: For some of the systems mentioned, most of the code weight is in callbacks, not hooks. (Eg, actions.) I don't know if it's safe to split those out, since we have no auto-loading for callbacks except manually for menu and theme.
Comment #4
catchComment #5
Crell commentedSo sun, you want to run a quick patch for the above? :-)
Comment #6
sunSure. Why not?
Comment #8
sunWrong stuff:
- hook_element_info + _alter -- uncached
Missing:
- New Image module hooks
- New File module hooks
- etc.
meh. For any reason, this does the opposite and gets slower. $module.build.inc is loaded on each page request.
Comment #10
sunRemoved the split into $module.build.inc, since that belongs into a separate issue anyway. This issue is about embedded support code for modules that may not be enabled at all, i.e. truly unused code, not something hypothetical or debatable in between.
Comment #11
catchSomehow I think block module is going to invoke its own implementations of its own hooks when it's enabled, and not when it's not, so that chunk needs to go.
Comment #12
Crell commentedcatch: I disagree. It's inconsistent for core modules to sometimes have optional hooks broken out and sometimes not. I'd rather have consistent organization in core.
Comment #13
catchWell then on any site using block module, which is currently 100% in D6 and likely to be in the high 90s in D7 for quite a while, you're going to have to load an extra include for no reason, on every request.
Comment #14
alexanderpas commentedthe block hooks are not optional in the block module ;)
my opinion on where hooks should live after this patch:
($module is the current module, $group is the group from hook_hook_info())
if $module defines $group or $module depends on the module that defines $group
- $module.module
else
- $module.$group.inc
callbacks may only be defined in $module.$group.inc if they are only used when the module that defines $group is enabled.
Comment #15
alexanderpas commentedWTF i didn't change that field
Comment #16
agentrickardUpdating to D8 and cross-referencing with #1107528: Stop using magic functions and force modules to declare the hooks they want to implement. Even if we don't move all core implementations, we should allow developers to decide in contrib.
Comment #17
sunThis issue is obsolete as we have properly auto-loaded code in D8.
Instead, we should sorta do the opposite: #2233261: Deprecate hook_hook_info groups, mark hook_hook_info() for deletion :-)