Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Found while profiling #1843746-46: Convert views/templates/views-view-field.tpl.php to Twig - this method gets called quite a bit when the same template is used multiple times in a request.
Patch coming after I do some initial profiling. This could potentially be submitted upstream as well.
Comment | File | Size | Author |
---|---|---|---|
#6 | 1979290-6.patch | 1.59 KB | soulston |
#1 | 1979290-1.patch | 1.62 KB | star-szr |
Comments
Comment #1
star-szrSomething like this?
Tested on a page that renders a node as well as 200 fields in a Views table via views-view-field.html.twig.
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=517896dc57c6e&...
Comment #2
Fabianx CreditAttribution: Fabianx commentedI like this!
Pretty straightforward code, has benchmarks to back it up, theme system things are called like 1000 times, so effect will be even higher (more like 25-30 ms).
6 kB more vs 4-5 ms saved is a really pretty deal!
Comments are nice, tests still pass, can't think of anything else.
It is free performance, lets take it :-).
=> RTBC
Comment #3
star-szrThanks for reviewing @Fabianx! I think this qualifies as a quick fix :)
Comment #4
alexpottNeeds to document the function params... and according the docs should be
Overrides
and notImplements
...Comment #5
soulston CreditAttribution: soulston commentedComment #6
soulston CreditAttribution: soulston commentedRe-rolled, thanks for the help @alexpott
This function is overriding so inherit the documentation using {@inheritdoc}.
Comment #7
catchIs there an upstream issue/pr for this?
Comment #8
star-szrI'll create a pull request later today (in about 4h).
Comment #9
star-szrHere's the PR: https://github.com/fabpot/Twig/pull/1071
Comment #10
catch#6: 1979290-6.patch queued for re-testing.
Comment #11
alexpottCommitted dced5e1 and pushed to 8.x. Thanks!
Obviously if the Twig PR lands we can rolls this back... nice work btw!
Comment #13
fabpot CreditAttribution: fabpot commentedJust for your information, the patch has been just merged upstream.
Comment #14
star-szrThanks @fabpot!
Comment #15
star-szrThis was reverted upstream due to a BC break:
https://github.com/fabpot/Twig/pull/1071#issuecomment-27481011
https://github.com/fabpot/Twig/issues/1251