778631 - use a persistent Template::Provider to avoid recompiling templates between page loads on mod_perl

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[:glob ✱]

Attachment: patch v1

the normal way for a template-toolkit app to run under mod_perl is to have a single instance of Template per process, and reuse that for each request (bug 343346).  after investigation this doesn't appear to be trivial.

a simple fix is to use a single Template::Provider per process.

Template::Provider is the object which finds and loads the templates from disk, compiles, and manages two caches of the compiled template -- one one disk (in data/template) and one in memory.

currently the memory cache keeps bugzilla from re-reading the compiled template from disk if a template is used more than once during the same request (eg bug/link.html.tmpl), however this memory cache is dropped at the end of each request.

this patch makes our provider per-request.

Comment 1

[Frédéric Buclin]

When a compiled template is used across requests, are you sure we are not passing the same data to different users?

Comment 2

[:glob ✱]

(In reply to Frédéric Buclin from comment #1)
> When a compiled template is used across requests, are you sure we are not
> passing the same data to different users?

yes; the part which is being cached is the compiled perl form of the template, not the generated html content.

Comment 3

[:glob ✱]

> this patch makes our provider per-request.

oops, that should be "per-process" not "per-request".

Comment 4

[David Lawrence [:dkl]]

Comment on attachment 647075 [details] [diff] [review]
patch v1

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

Looks good and works as expected. My timings show some decrease in overall time for loading a series of heavy pages. r=dkl

Comment 5

[:glob ✱]

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

the release notes need to mention an increase in memory requirements, as well as updates to templates taking up to an hour to become active (both mod_perl only).

Comment 6

[:glob ✱]

this caused burning on tinderbox:

Removing existing compiled templates...
Precompiling templates...file error - account/auth/login-small.html.tmpl: not found

i've backed this out:

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

and will investigate.

Comment 7

[:glob ✱]

Attachment: patch v2

bugzilla uses the template's INCLUDE_PATH to ensure the correct language file is loaded, so we need a separate provider object cached for each INCLUDE_PATH.

during template compilation, the INCLUDE_PATH is changed from using the default list to one which only contains the parent of the file being compiled.  patch v1 would have cached only the first directory, causing the failures.

both issues are resolved by using the INCLUDE_PATH as a key for multiple provider objects.  during compilation the provider objects created with the single-directory INCLUDE_PATHs are deleted after use.

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 649650 [details] [diff] [review]
patch v2

Seems to work well in my testing and still gives a small performance boost. Unfortunately for BMO at least it does create some large provider keys, i.e.:

/home/dkl/devel/htdocs/mod_perl/extensions/BMO/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/BrowserID/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/BzAPI/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/ComponentWatching/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/ContributorEngagement/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/FlagTypeComment/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/GuidedBugEntry/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/InlineHistory/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/LastResolved/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/REMO/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/RequestWhiner/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/SecureMail/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/SiteMapIndex/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/Splinter/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/TagNewUsers/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/TryAutoLand/template/en/default:/home/dkl/devel/htdocs/mod_perl/extensions/Voting/template/en/default:/home/dkl/devel/htdocs/mod_perl/template/en/default

It is depending on the number of extensions installed which BMO has quite a few so probably not that bad for normal installations. I don't remember there being a limit on hash key size either so that shouldn't be an issue.

r=dkl

Comment 9

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #8)
> It is depending on the number of extensions installed which BMO has quite a
> few so probably not that bad for normal installations. I don't remember
> there being a limit on hash key size either so that shouldn't be an issue.

it won't take longer for longer keys to get matched: it will take infinitesimally longer to hash the key, but that's all.  (perl use the hash of the key to find the right chain, not the key value itself -- see http://www.perl.com/pub/2002/10/01/hashes.html for more info).

Comment 10

[Frédéric Buclin]

So if I understand correctly, bmo has something like 20 or 30 Template::Provider objects? How much memory does this consume?

Comment 11

[:glob ✱]

(In reply to Frédéric Buclin from comment #10)
> So if I understand correctly, bmo has something like 20 or 30
> Template::Provider objects? How much memory does this consume?

no, we'll have one provider object as we only have one language installed.

there will be a persistent provider for each unique INCLUDE_PATH.  this path only changes when the language is changed (eg. "template/en/default:template/fr/default" vs "template/fr/default:template/en/default").  the number of installed extensions increases the length of the path, but not the number of different paths.

i wasn't able to see any noticeable differences when measuring memory usage on my development system (but it was very noisy so difficult to tell).

Comment 12

[:glob ✱]

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

Comment 13

[Frédéric Buclin]

Added to relnotes for 4.4.