1352908 - Optimize certain filters in the template compiler

Status: NEW

(bugzilla.mozilla.org :: General, enhancement)

Description

[Dylan Hardison [:dylan] (he/him)]

Optimize certain filters in the template compiler

[% ... FILTER none %] is optimized away. There is no cost for using it.
[% ... FILTER html %] is optimized to a direct call to
Bugzilla::Util::html_quote, saving a method call to $stash->filter.
[% ... FILTER null %] avoids a filter call and always results in ''

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Attachment: PR

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1352908_1.patch

Comment 3

[:glob ✱]

Comment on attachment 8853975 [details] [diff] [review]
1352908_1.patch

Review of attachment 8853975 [details] [diff] [review]:
-----------------------------------------------------------------

nice optimisation!

::: Bugzilla/Template/Directive.pm
@@ +5,5 @@
> +# This Source Code Form is "Incompatible With Secondary Licenses", as
> +# defined by the Mozilla Public License, v. 2.0.
> +
> +# This exists to implement the template-before_process hook.
> +package Bugzilla::Template::Directive;

i suspect this exists for a different reason.

@@ +15,5 @@
> +use base qw(Template::Directive);
> +
> +sub filter {
> +    package Template::Directive;
> +    our ($PRETTY, $OUTPUT);

i'd prefer this to be written as $PRETTY = $Template::Directive::PRETTY, same for OUTPUT

Comment 4

[Dylan Hardison [:dylan] (he/him)]

I finally got to cleaning up this code, and I've also profiled the impact it has. For 257 page visits, the difference is:

without patch:
# spent 2.53s (1.64+890ms) within Bugzilla::Template::Context::filter which was called 381645 times, avg 7µs/call:

with patch:
# spent 470ms (186+285) within Bugzilla::Template::Context::filter which was called 37008 times, avg 13µs/call:

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Attachment: PR

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Attachment: user-error.patch

Show casing the difference in the generated code

Comment 7

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Is this still being worked on?

Comment 8

[Dylan Hardison [:dylan] (he/him)]

Yeah, it was one of the things I worked on over the break. It seemed to break tests, and I was bisecting that problem.
Difficult because tests take so long to run.