With hook_hook_info(), D7 offers a very easy way to implement lazy loading for module-defined API hooks.

I figured especially for something like hook_flag_default_flags(), which doesn't have to be called all the time, it would be a nice idea to offer people to place it in a separate mymodule.flag.inc file which is autoloaded on demand.

CommentFileSizeAuthor
#8 1524816.flag_.hook-hook-info.patch7.85 KBjoachim

Comments

joachim’s picture

Status: Active » Postponed

That hook is actually invoked every page load that flag_get_flags() is called, which is going to be on any response to a flag menu item, and any entity view of an entity that's potentially flaggable: nodes, comments, users, and more.

So right now AFAICT this would just add file includes to Drupal. If we added database caching to flag_get_flags() then moving hook_flag_default_flags() to .inc files would be a good idea, yes. Marking this as postponed.

joachim’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
Status: Postponed » Active

Moving to the 3.x branch, but could potentially be backported to 2.x.

joachim’s picture

> mymodule.flag.inc

And hence for flag itself, we'd have flag.flag.inc

We have quite a few hooks that are either rarely called, or can be made so with some caching.

joachim’s picture

Rough plan:

MODULE.flag.inc:
- hook_flag_definitions
- hook_flag_definitions_alter
- hook_flag_link_types
- hook_flag_link_types_alter

MODULE.flag_default.inc:
- hook_flag_default_flags

Though all the above need some database caching setting up first.

joachim’s picture

Status: Active » Postponed

Also, do this after #1699756: rename info hooks (eg hook_flag_definitions) to hook_foo_info(), so that issue has fewer string changes to make.

joachim’s picture

Updated plan now the hook name have changed:

MODULE.flag.inc:
- hook_flag_type_info
- hook_flag_type_info_alter
- hook_flag_link_type_info
- hook_flag_link_type_info_alter

MODULE.flag_default.inc:
- hook_flag_default_flags

joachim’s picture

Status: Postponed » Fixed
StatusFileSize
new7.85 KB

Committed this patch.

See #1735556: [meta] flag performance: db caching, hook_hook_info() and .inc files, where it received testing as part of a meta issue.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.