Patchwork [3,of,8] templater: drop extension point of engine classes (API)

login
register
mail settings
Submitter Yuya Nishihara
Date June 14, 2018, 3:40 p.m.
Message ID <dd8aa381ae51a9b72107.1528990828@mimosa>
Download mbox | patch
Permalink /patch/32133/
State Accepted
Headers show

Comments

Yuya Nishihara - June 14, 2018, 3:40 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1525312736 -32400
#      Thu May 03 10:58:56 2018 +0900
# Node ID dd8aa381ae51a9b72107cd11bb2af0b914ca6411
# Parent  7e09d67f3c253714401b4a02df741412ccab3e12
templater: drop extension point of engine classes (API)

I don't think this would ever be used by third-party extensions, as we've
heavily changed both the templater internals and the syntax since then.

The main reason of removing this API is that I want to move the parsing
function from the engine to the templater class so that we can peek keywords
and functions used in a user template. This change also removes reference
cycle between the templater and the engine.
via Mercurial-devel - June 20, 2018, 10:23 p.m.
On Thu, Jun 14, 2018 at 8:40 AM Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1525312736 -32400
> #      Thu May 03 10:58:56 2018 +0900
> # Node ID dd8aa381ae51a9b72107cd11bb2af0b914ca6411
> # Parent  7e09d67f3c253714401b4a02df741412ccab3e12
> templater: drop extension point of engine classes (API)
>

This series (starting with this patch) makes our default Google-internal
log command (an alias with lots of templating) noticeably slower. Before
this patch, it runs in about ~3.6s. After the next patch ("templater: parse
template string to tree by templater class"), it runs in ~5.6s. (With this
patch, it runs in ~10.7s.) As I said, it uses lots of templating, so I
guess it's the lack of caching of the parsed template that you mention in
the next patch that's the issue.

It's easy to reproduce the slowdown with a very artificial template like
this (slows down from ~1s to ~2.3s):
./hg log -r 'not public()' -T "$(yes '{rev}' | head -100)" > /dev/null

In case it's helpful, I also timed a very simple template:
./hg log -r 'not public()' -T '{rev}\n' > /dev/null
Before this patch, that command takes ~0.5s. With this patch, it takes
~3.5s. With the next patch, it's back to ~0.5s.
Yuya Nishihara - June 21, 2018, 11:51 a.m.
On Wed, 20 Jun 2018 15:23:51 -0700, Martin von Zweigbergk wrote:
> On Thu, Jun 14, 2018 at 8:40 AM Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1525312736 -32400
> > #      Thu May 03 10:58:56 2018 +0900
> > # Node ID dd8aa381ae51a9b72107cd11bb2af0b914ca6411
> > # Parent  7e09d67f3c253714401b4a02df741412ccab3e12
> > templater: drop extension point of engine classes (API)
> >
> 
> This series (starting with this patch) makes our default Google-internal
> log command (an alias with lots of templating) noticeably slower. Before
> this patch, it runs in about ~3.6s. After the next patch ("templater: parse
> template string to tree by templater class"), it runs in ~5.6s. (With this
> patch, it runs in ~10.7s.) As I said, it uses lots of templating, so I
> guess it's the lack of caching of the parsed template that you mention in
> the next patch that's the issue.

Appears that I've disabled the cache of compiled template functions by mistake.
I'll include a fix in the next batch. Thanks.

Patch

diff --git a/contrib/python3-whitelist b/contrib/python3-whitelist
--- a/contrib/python3-whitelist
+++ b/contrib/python3-whitelist
@@ -468,7 +468,6 @@  test-symlink-placeholder.t
 test-symlinks.t
 test-tag.t
 test-tags.t
-test-template-engine.t
 test-template-filters.t
 test-treemanifest.t
 test-ui-color.py
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -718,8 +718,6 @@  class engine(object):
             mapping = extramapping
         return templateutil.flatten(self, mapping, func(self, mapping, data))
 
-engines = {'default': engine}
-
 def stylelist():
     paths = templatepaths()
     if not paths:
@@ -777,10 +775,7 @@  def _readmapfile(mapfile):
                                        conf.source('templates', key))
             cache[key] = unquotestring(val)
         elif key != '__base__':
-            val = 'default', val
-            if ':' in val[1]:
-                val = val[1].split(':', 1)
-            tmap[key] = val[0], os.path.join(base, val[1])
+            tmap[key] = os.path.join(base, val)
     aliases.extend(conf['templatealias'].items())
     return cache, tmap, aliases
 
@@ -815,7 +810,6 @@  class templater(object):
         self._resources = resources
         self._aliases = aliases
         self._minchunk, self._maxchunk = minchunk, maxchunk
-        self._ecache = {}
 
     @classmethod
     def frommapfile(cls, mapfile, filters=None, defaults=None, resources=None,
@@ -835,13 +829,13 @@  class templater(object):
         '''Get the template for the given template name. Use a local cache.'''
         if t not in self.cache:
             try:
-                self.cache[t] = util.readfile(self._map[t][1])
+                self.cache[t] = util.readfile(self._map[t])
             except KeyError as inst:
                 raise templateutil.TemplateNotFound(
                     _('"%s" not in template map') % inst.args[0])
             except IOError as inst:
                 reason = (_('template file %s: %s')
-                          % (self._map[t][1],
+                          % (self._map[t],
                              stringutil.forcebytestr(inst.args[1])))
                 raise IOError(inst.args[0], encoding.strfromlocal(reason))
         return self.cache[t]
@@ -857,16 +851,8 @@  class templater(object):
     def generate(self, t, mapping):
         """Return a generator that renders the specified named template and
         yields chunks"""
-        ttype = t in self._map and self._map[t][0] or 'default'
-        if ttype not in self._ecache:
-            try:
-                ecls = engines[ttype]
-            except KeyError:
-                raise error.Abort(_('invalid template engine: %s') % ttype)
-            self._ecache[ttype] = ecls(self.load, self._filters, self.defaults,
-                                       self._resources, self._aliases)
-        proc = self._ecache[ttype]
-
+        proc = engine(self.load, self._filters, self.defaults, self._resources,
+                      self._aliases)
         stream = proc.process(t, mapping)
         if self._minchunk:
             stream = util.increasingchunks(stream, min=self._minchunk,
diff --git a/tests/test-template-engine.t b/tests/test-template-engine.t
deleted file mode 100644
--- a/tests/test-template-engine.t
+++ /dev/null
@@ -1,64 +0,0 @@ 
-
-  $ cat > engine.py << EOF
-  > 
-  > from mercurial import (
-  >     pycompat,
-  >     templater,
-  >     templateutil,
-  > )
-  > 
-  > class mytemplater(templater.engine):
-  >     def _load(self, t):
-  >         return self._loader(t)
-  > 
-  >     def process(self, t, map):
-  >         tmpl = self._load(t)
-  >         props = self._defaults.copy()
-  >         props.update(map)
-  >         for k, v in props.items():
-  >             if b'{{%s}}' % k not in tmpl:
-  >                 continue
-  >             if callable(v) and getattr(v, '_requires', None) is None:
-  >                 props = self._resources.copy()
-  >                 props.update(map)
-  >                 v = v(**pycompat.strkwargs(props))
-  >             elif callable(v):
-  >                 v = v(self, props)
-  >             v = templateutil.stringify(self, props, v)
-  >             tmpl = tmpl.replace(b'{{%s}}' % k, v)
-  >         yield tmpl
-  > 
-  > templater.engines[b'my'] = mytemplater
-  > EOF
-  $ hg init test
-  $ echo '[extensions]' > test/.hg/hgrc
-  $ echo "engine = `pwd`/engine.py" >> test/.hg/hgrc
-  $ cd test
-  $ cat > mymap << EOF
-  > changeset = my:changeset.txt
-  > EOF
-  $ cat > changeset.txt << EOF
-  > {{rev}} {{node}} {{author}}
-  > EOF
-  $ hg ci -Ama
-  adding changeset.txt
-  adding mymap
-  $ hg log --style=./mymap
-  0 97e5f848f0936960273bbf75be6388cd0350a32b test
-
-  $ cat > changeset.txt << EOF
-  > {{p1rev}} {{p1node}} {{p2rev}} {{p2node}}
-  > EOF
-  $ hg ci -Ama
-  $ hg log --style=./mymap
-  0 97e5f848f0936960273bbf75be6388cd0350a32b -1 0000000000000000000000000000000000000000
-  -1 0000000000000000000000000000000000000000 -1 0000000000000000000000000000000000000000
-
-invalid engine type:
-
-  $ echo 'changeset = unknown:changeset.txt' > unknownenginemap
-  $ hg log --style=./unknownenginemap
-  abort: invalid template engine: unknown
-  [255]
-
-  $ cd ..