Spin-off from #659788: [meta issue] theme('field') is too slow, which is inhibiting its more widespread use. One of several patches aimed at optimizing theme('field') while still leaving it as a template implementation rather than a function implementation. Will provide benchmarks once #538164: Comment body as field lands, since that's the driving use-case.

CommentFileSizeAuthor
optimize-drupal_discover_template.patch2.27 KBeffulgentsia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, , failed testing.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

+1 for this optimization. subscribe.

yched’s picture

yched CreditAttribution: yched commented

Adding a static cache definitely makes sense. I'd be interested in the bench results.

- I'm wondering about the performance impact of using a potentially fairly long string as an array key
$key = implode(',', $paths) . ':' . implode(',', $suggestions) . ':' . $extension;

- Should probably use a comment to explain why it doesn't use drupal_static() ?

- Isn't array_key_exists() notably slow ?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Subscribing.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

Now that comment body as field is in, I'll post benchmarks soon. But, I prefer #678714: Unify use of theme hook / template suggestions, fix clobbering problems, and improve suggestion discovery performance, which would make this one irrelevant.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

#678714: Unify use of theme hook / template suggestions, fix clobbering problems, and improve suggestion discovery performance is in. Close this?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Closed (duplicate)

Yep.