Remove 'includes' support from hook_theme()

Created on 3 June 2011, over 14 years ago
Updated 17 January 2025, 9 months ago

_theme_process_registry() includes any include file (could be a template or foo.admin.inc) when building the registry. It looks like including the file isn't necessary.

I traced the include back to #168028: both theme functions and templates may need include files β†’ , when that patch went in hook_theme() and especially include file/template handling was very different, and as far as I can tell it's no longer needed. Originally the include had a comment 'just to be safe', this was lost in the past three years.

This saves at least 40ms off a registry rebuild, but more importantly 5mb of memory, with just core modules enabled. Sites with lots of modules, lots of includes and lots of templates are going to see considerably bigger savings.

As well as this, all those included files, many of which are never going to be used if they're templates, will pollute opcode caches - potentially causing evictions etc. - so this should help on cold starts a fair bit.

Theme tests pass but didn't run the whole suite.

If I run drush cc all then hit the front page without apc enabled, that requires 45mb of memory with all the caches being rebuilt.

I confirmed that the total memory usage with this patch dropped to 41mb. So if these includes really aren't needed, that may end up helping with issues like #1008362: 32M is sometimes not enough memory to install Drupal 7 β†’ , module page submit - anywhere else where a single request can end up building all global caches.

If this doesn't break things horribly, it may win the award for most kilobytes of memory saved per line of patch ;)

β†’

πŸ“Œ Task
Status

Postponed

Version

11.0 πŸ”₯

Component

theme system

Created by

πŸ‡¬πŸ‡§United Kingdom catch

Live updates comments and jobs are added and updated live.
  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    πŸ“Œ Handle module preprocess functions Active should make this pretty straight-forward. Once we converted the remaining preprocess functions in .inc files in core like system and views, we should be able to trigger deprecations if this is set.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    ThemeCommonElements uses it too, but that should be able to be cleaned up too.

  • Merge request !11808Resolve #1177762 "Remove includes support" β†’ (Open) created by berdir
  • Pipeline finished with Failed
    6 months ago
    Total: 187s
    #471861
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Still postponed, but wanted to have a try, all we have to do now is trigger a deprecation if one of the two keys is set, started a MR based on πŸ“Œ [pp-1] Convert Template Preprocess hooks to OOP equivalent Postponed: needs info which removes those in system module and a quickfix for authorize report that I might move to πŸ“Œ Convert Convert Template Preprocess hooks in core/includes Active , there shouldn't be too much left after that.

  • Pipeline finished with Failed
    6 months ago
    Total: 554s
    #471873
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    Blocker has been committed

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Still postponed on at 50% of πŸ“Œ [pp-1] Convert Template Preprocess hooks to OOP equivalent Postponed: needs info , template_preprocess is actually the main thing here, module preprocess aren't really in those files. I'm aggregating the relevant ones from the meta to check what's left, but keeping at postponed.

  • Pipeline finished with Failed
    3 months ago
    Total: 198s
    #549836
  • Pipeline finished with Failed
    3 months ago
    Total: 751s
    #549843
  • Pipeline finished with Failed
    3 months ago
    #557426
  • Pipeline finished with Success
    3 months ago
    Total: 646s
    #557430
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    This is still postponed and contains 4 or so other issues and a bunch of manual fixes, but with all that, it is now green. We might need to add a BC test though for this. could be combined with a BC test for template_preprocess.

  • Pipeline finished with Failed
    15 days ago
    Total: 395s
    #613764
  • Pipeline finished with Failed
    15 days ago
    Total: 668s
    #613785
  • Pipeline finished with Failed
    15 days ago
    Total: 491s
    #613919
  • Pipeline finished with Failed
    15 days ago
    Total: 445s
    #613946
  • Pipeline finished with Failed
    4 days ago
    Total: 421s
    #624940
  • Pipeline finished with Failed
    4 days ago
    Total: 204s
    #624956
  • Pipeline finished with Failed
    4 days ago
    Total: 215s
    #624961
  • Pipeline finished with Success
    4 days ago
    Total: 7448s
    #624966
Production build 0.71.5 2024