The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.So this may be treading back towards the registry, but currently it's impossible for Token module to easily tell the module hook system that it has the menu, profile, and field implementations for tokens. Currently with module_implements we cache discovered implementations like this:
$implementations[hook][module] = FALSE; // Means the hook is located in the module file. Since all module files are loaded on full bootstrap, we don't care where the hook is located.
$implementations[hook][module] = 'group' // Means the hook is located in module.group.inc inside the module's root directory. This group values is taken from hook_hook_info().
Now if we stored the actual file location instead of 'group', it could allow modules to easily add core module support for hooks. In the case of token.module, we'd want to do something like this:
function token_module_implements_alter(&$implementations, $hook) {
if ($hook == 'tokens' || $hook == 'token_info') {
$modules = array('menu', 'profile', 'field');
foreach ($modules as $module) {
if (module_exists($module) && $file = module_load_include('inc', 'token', $module . '.tokens')) {
$implementations[$module] = $file;
// Ensure this gets cached since we made changes.
$implementations['#write_cache'] = TRUE;
}
}
}
}
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | drupal.module-implements-file.8.patch | 2.88 KB | sun |
| #3 | 825854-module-implements-file-D7.patch | 2.91 KB | dave reid |
| #1 | 825854-module-implements-file-D7.patch | 1.84 KB | dave reid |
Comments
Comment #1
dave reidPatch attached for review.
Comment #3
dave reidRevised to fix the one test failure that was anticipating the group name.
Comment #4
dave reidAdding tags for the D7 modules this would make life easier for.
Comment #5
damienmckennaThe logic makes sense, the code is simple & clean. +1
How many reviews are needed for it to be RBTC?
Comment #6
retester2010 commented#3: 825854-module-implements-file-D7.patch queued for re-testing.
Comment #8
sunStraight re-roll against HEAD. Didn't verify the changes, but overall, they make sense to me.
Comment #10
catchSomeone needs to profile this patch with xhprof, with a lot of modules enabled, to see if there's any noticeable memory impact from storing the file path. I would hope there isn't but let's not find out after this gets in.
Comment #11
dave reid#8: drupal.module-implements-file.8.patch queued for re-testing.
Comment #13
dave reidI don't think I'm in a position to be able to do that. I can apache ab it if that will help?
Comment #14
catchab won't tell you memory usage. You could var_export() the cache entry and compare file size maybe - but it needs to be on a large site with lots of hook implementations to be meaningful.
Comment #15
dave reidYeah the problem is that core does not use hook_hook_info() very much, so that will be kinda hard to do.
Comment #16
catchThis isn't about hook_hook_info(), it's just about how much the array grows when you store filename for every module instead of FALSE. So the memory issue is going to be most apparent when hook_hook_info() isn't used.
My concern here is that to save memory from loading files, we are increasing the metadata loaded into memory on every request - so if there are only a few core hooks being implemented here, then it is likely that the extra memory from the cache size is going to outweigh any gains.
This will especially be the case if you don't have contrib modules installed doing the hook_module_implements_alter() trick (or not many) - because that memory cost is going to hit you whether or not that gets used or not.
Comment #17
dave reidThe size of this cache will only increase if hook_hook_info() is used and the module's hook is located in module-name.hook-group.inc. If the hook is located in the main .module file, then the value stored will not change (will still be FALSE).
Comment #18
catchAh OK.
Just made the title of the issue a bit longer to reflect this.
Reviewing the code I see that's what you're doing, and that should have minimal impact, so this is probably fine.
Comment #19
jhedstromThis would probably need to wait for at least 8.1.
Comment #35
smustgrave commentedClosing as it appears hook_info is being deprecated in https://www.drupal.org/project/drupal/issues/2233261