Bug 20975

Created attachment 76230 [details] [review]
AutoEscaping - no filter

for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done
perl benchmark_AutoEscaping.pl  0.20s user 0.00s system 98% cpu 0.208 total
perl benchmark_AutoEscaping.pl  0.19s user 0.02s system 98% cpu 0.214 total
perl benchmark_AutoEscaping.pl  0.18s user 0.02s system 98% cpu 0.204 total
perl benchmark_AutoEscaping.pl  0.18s user 0.02s system 98% cpu 0.202 total
perl benchmark_AutoEscaping.pl  0.18s user 0.02s system 97% cpu 0.205 total

Created attachment 76231 [details] [review]
AutoEscaping - TT html filters

for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done
perl benchmark_AutoEscaping.pl  0.30s user 0.09s system 95% cpu 0.414 total
perl benchmark_AutoEscaping.pl  0.27s user 0.02s system 97% cpu 0.295 total
perl benchmark_AutoEscaping.pl  0.28s user 0.02s system 99% cpu 0.307 total
perl benchmark_AutoEscaping.pl  0.30s user 0.02s system 98% cpu 0.320 total
perl benchmark_AutoEscaping.pl  0.27s user 0.02s system 99% cpu 0.290 total

Created attachment 76232 [details] [review]
AutoEscaping - Template::Stash::AutoEscaping (escape all)

for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done
perl benchmark_AutoEscaping.pl  9.67s user 5.08s system 99% cpu 14.837 total
perl benchmark_AutoEscaping.pl  9.00s user 4.10s system 99% cpu 13.167 total
perl benchmark_AutoEscaping.pl  9.24s user 4.97s system 99% cpu 14.263 total
perl benchmark_AutoEscaping.pl  9.91s user 4.19s system 99% cpu 14.188 total
perl benchmark_AutoEscaping.pl  8.86s user 4.44s system 99% cpu 13.347 total

Created attachment 76233 [details] [review]
AutoEscaping - Do not escape using ignore_escape

The variable are not escaped (let's suppose it's what we want)

for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done
perl benchmark_AutoEscaping.pl  7.37s user 4.12s system 99% cpu 11.557 total
perl benchmark_AutoEscaping.pl  7.15s user 4.24s system 99% cpu 11.438 total
perl benchmark_AutoEscaping.pl  6.99s user 3.70s system 99% cpu 10.751 total
perl benchmark_AutoEscaping.pl  6.32s user 3.76s system 99% cpu 10.120 total
perl benchmark_AutoEscaping.pl  6.32s user 3.71s system 99% cpu 10.057 total

Created attachment 76234 [details] [review]
AutoEscaping - Do not escape using .raw

Same as before, vars will not be escaped

for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done
perl benchmark_AutoEscaping.pl  7.43s user 3.92s system 99% cpu 11.400 total
perl benchmark_AutoEscaping.pl  6.56s user 3.75s system 99% cpu 10.377 total
perl benchmark_AutoEscaping.pl  6.11s user 4.08s system 99% cpu 10.233 total
perl benchmark_AutoEscaping.pl  7.02s user 3.93s system 99% cpu 10.984 total
perl benchmark_AutoEscaping.pl  6.91s user 3.70s system 99% cpu 10.664 total

Created attachment 76235 [details] [review]
AutoEscaping - Koha::Template::AutoEscaping

for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done
perl benchmark_AutoEscaping.pl  0.34s user 0.07s system 96% cpu 0.425 total
perl benchmark_AutoEscaping.pl  0.35s user 0.04s system 99% cpu 0.387 total
perl benchmark_AutoEscaping.pl  0.35s user 0.04s system 99% cpu 0.396 total
perl benchmark_AutoEscaping.pl  0.50s user 0.03s system 99% cpu 0.532 total
perl benchmark_AutoEscaping.pl  0.38s user 0.02s system 99% cpu 0.400 total

Koha::Template::AutoEscaping is a naive adaptation of Template::Stash::AutoEscaping. I have tried to simplify the code to remove what will not be useful for us.

The performances are good, I guess it helps to avoid 1 object creation (::Escaped::HTML) by variable.

But... it does not work. If you are applying the last patch on top of bug 13618 you will notice that variables are double-escaped.

(In reply to Jonathan Druart from comment #7)
> The performances are good, I guess it helps to avoid 1 object creation
> (::Escaped::HTML) by variable.

It's actually worse than that. Template::Stash::AutoEscaping creates an object for each string concatenation, and Template::Toolkit concatenates stuff to its internal buffer all the time.
I ran benchmark_AutoEscaping.pl (using Template::Stash::AutoEscaping) with NYTProf and it spends most of the time in Template::Stash::AutoEscaping::Escaped::Base::concat (63% of total time) where it creates 50000 new objects. But apparently most of the time here is spent doing simple string concatenation.
(not so) fun fact: the "HTML-escaping" process represents less than 3% of concat in terms of time spent.

I don't know how (or if) this can be improved, but I think the time loss is not acceptable, and unless we find a better solution we should keep using html filter manually (and probably add a rule in coding guidelines)

Note that we can use TT filters as blocks, like this:

[% FILTER html %]
  [% this_will_be_escaped %] and [% this.will(be).escaped | too %]
[% END %]

Maybe this could reduce the hassle of writing "|html" everywhere

No longer valid, new version of bug 13618 has been submitted.

Thanks for the feedback Julian!