795650 - Cache the HTML::Scrubber object for improved performance

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frédéric Buclin]

Every time a template calls |FILTER html_light|, a new HTML::Scrubber object is created with the exact same data. When viewing a list of 4000 users in editusers.cgi, my installation spends 4.75 s in html_light_quote(), most of which is used to create the HTML::Scrubber object again and again. When storing the object and reusing it, it now spends 1.35 s in html_light_quote(), most of which is used to sanitize the data. This is a 3.4 seconds win!

There are many places where we call |FILTER html_light| several times in a single template, and they will all benefit from this improvement.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[:glob ✱]

Comment on attachment 666261 [details] [diff] [review]
patch, v1

this looks like a good improvement, but i see no reason why this shouldn't be cached pre-process instead of a per-request.

now's probably a good time to create a Bugzilla->process_cache and utilise that.

Comment 3

[Frédéric Buclin]

Comment on attachment 666261 [details] [diff] [review]
patch, v1

Sorry, but I don't want to create Bugzilla->process_cache in this bug. The goal of this patch/bug is to cache the HTML::Scrubber object using the existing Bugzilla->request_cache infrastructure, i.e. on a per-page basis instead of calling it tens of thousands time in a single page load as we do currently. Your per-process proposal should be part of a separate bug, assuming there is other relevant code which could be cached cross-requests too. I want to take this patch for 4.4, and I think it's a bit too late to implement Bugzilla->process_cache for 4.4 now.

Comment 4

[:glob ✱]

Comment on attachment 666261 [details] [diff] [review]
patch, v1

(In reply to Frédéric Buclin from comment #3)
> Sorry, but I don't want to create Bugzilla->process_cache in this bug.

ok, i'll file a separate bug for that.

for now, please use an "our" variable in the util module to cache the scrubber object.  there's no reason to limit this optimisation to per-request when it's trivial to make it per-process.

Comment 5

[Frédéric Buclin]

Attachment: patch, v2

s/request_cache/process_cache/g

Comment 6

[:glob ✱]

Comment on attachment 668397 [details] [diff] [review]
patch, v2

r=glob

Comment 7

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Util.pm
Committed revision 8417.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/Util.pm
Committed revision 8411.