Repository: ckan/ckan
Issue category: Performance Issue

Issue:
snippet performance issue
**CKAN version**
all

**Describe the bug**
Heavy use of template `{% snippet %}`  slows page rendering times compared to alternatives like macros or `{% include %}`

**Steps to reproduce**
Compare the rendering time of a page using snippets with one that uses macros or `{% include %}` to display the same information. The activity feed, the search results page and customized dataset forms when using ckanext-scheming are examples of pages with lots of snippets.

**Expected behavior**
Render time for `{% snippet %}` should be similar to macros and `{% include %}`

**Additional details**
`{% snippet %}` is a great tag worth saving, not just for compatibility.

Unlike `{% include %}` snippets support passing parameters, and unlike macros snippet parameters can be added over time without causing errors. Snippets are easier for developers to use than deeply extended templates with blocks selectively overridden.

The performance issue may be caused by the way jinja2 converts templates into byte code - a template passed to `render()` needs to be interpreted and can't use the byte code cache, unlike extended templates or imported macros. Our `{% snippet %}` tag calls `render()` returning the resulting string so snippets are re-interpreted every time they are used, even if it's the exact same snippet used in a loop. 

A possible fix is to have the snippet tag automatically convert snippets into a macro with the parameters passed by the caller. The macro-ified snippets could be generated with a dynamic template loader instead of creating them on the filesystem.  The snippet tag would convert itself into an `{% import %}` and a call to the macro generated.


Hints:
@smotornyuk mentioned that current jinja2 now has an `{% include … without context %}` that we could have our snippet tag emit instead of calling render and inserting the contents. Should be an easy win.

Patch:
diff --git a/changes/6146.feature b/changes/6146.feature
new file mode 100644
index 00000000000..e40b88d474f
--- /dev/null
+++ b/changes/6146.feature
@@ -0,0 +1,1 @@
+Render snippets faster through better use of existing jinja2 tags. Use ``{% snippet 'path/to/snippet.html', arg1=test %}`` instead of ``{{ h.snippet('path/to/snippet.html', arg1=test) }}`` in templates for better performance.
diff --git a/ckan/config/middleware/flask_app.py b/ckan/config/middleware/flask_app.py
index 32825bb6896..5b59d0016de 100644
--- a/ckan/config/middleware/flask_app.py
+++ b/ckan/config/middleware/flask_app.py
@@ -21,7 +21,6 @@
     Unauthorized,
     Forbidden
 )
-from werkzeug.local import LocalProxy
 
 from flask_babel import Babel
 
@@ -99,14 +98,6 @@ def __call__(self, environ: Any, start_response: Any):
         return self.app(environ, start_response)
 
 
-def _ungettext_alias():
-    u'''
-    Provide `ungettext` as an alias of `ngettext` for backwards
-    compatibility
-    '''
-    return dict(ungettext=ungettext)
-
-
 def make_flask_stack(conf: Union[Config, CKANConfig]) -> CKANApp:
     """ This has to pass the flask app through all the same middleware that
     Pylons used """
@@ -182,6 +173,15 @@ def make_flask_stack(conf: Union[Config, CKANConfig]) -> CKANApp:
     app.jinja_env.filters['empty_and_escape'] = \
         jinja_extensions.empty_and_escape
 
+    # globals work in imported and included templates (like snippets)
+    # whereas context processors do not
+    app.jinja_env.globals.update({
+        'h': h.helper_functions,
+        'ungettext': ungettext,
+        'current_user': current_user,
+        'c': g,  # backwards compat. with old Pylons templates
+    })
+
     # Common handlers for all requests
     #
     # flask types do not mention that it's possible to return a response from
@@ -189,12 +189,6 @@ def make_flask_stack(conf: Union[Config, CKANConfig]) -> CKANApp:
     app.before_request(ckan_before_request)
     app.after_request(ckan_after_request)
 
-    # Template context processors
-    app.context_processor(helper_functions)
-    app.context_processor(c_object)
-
-    app.context_processor(_ungettext_alias)
-
     # Babel
     _ckan_i18n_dir = i18n.get_ckan_i18n_dir()
 
@@ -408,20 +402,6 @@ def ckan_after_request(response: Response) -> Response:
     return response
 
 
-def helper_functions() -> dict[str, h.HelperAttributeDict]:
-    u'''Make helper functions (`h`) available to Flask templates'''
-    if not h.helper_functions:
-        h.load_plugin_helpers()
-    return dict(h=h.helper_functions)
-
-
-def c_object() -> dict[str, LocalProxy[Any]]:
-    u'''
-    Expose `c` as an alias of `g` in templates for backwards compatibility
-    '''
-    return dict(c=g)
-
-
 class CKAN_AppCtxGlobals(_AppCtxGlobals):  # noqa
 
     '''Custom Flask AppCtxGlobal class (flask.g).'''
diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py
index a2af1644638..14a9ba2665a 100644
--- a/ckan/lib/helpers.py
+++ b/ckan/lib/helpers.py
@@ -1746,8 +1746,9 @@ def dump_json(obj: Any, **kw: Any) -> str:
 
 @core_helper
 def snippet(template_name: str, **kw: Any) -> str:
-    ''' This function is used to load html snippets into pages. keywords
-    can be used to pass parameters into the snippet rendering '''
+    '''
+    Use {% snippet %} tag instead for better performance.
+    '''
     import ckan.lib.base as base
     return base.render_snippet(template_name, **kw)
 
diff --git a/ckan/lib/jinja_extensions.py b/ckan/lib/jinja_extensions.py
index ce2608f1e30..a17b1504a53 100644
--- a/ckan/lib/jinja_extensions.py
+++ b/ckan/lib/jinja_extensions.py
@@ -9,11 +9,11 @@
 from jinja2 import nodes
 from jinja2 import loaders
 from jinja2 import ext
+from jinja2.parser import Parser
 from jinja2.exceptions import TemplateNotFound
 from jinja2.utils import open_if_exists
 from markupsafe import escape
 
-import ckan.lib.base as base
 import ckan.lib.helpers as h
 from ckan.common import config
 from markupsafe import Markup
@@ -197,6 +197,18 @@ def get_source(self, environment: Any, template: str) -> Any:
             searchpaths = self.searchpath[index + 1:]
         else:
             searchpaths = self.searchpath
+        # § snippet wrapper
+        smatch = re.match(r'([^"]+)§(\w+(?:,\w+)*)?([.]\w+)$', template)
+        if smatch:
+            # check for TemplateNotFound on real template
+            args = ',' + smatch[2] if smatch[2] else ''
+            return (
+                f'{{% macro snippet(_template{args}) %}}'
+                f'{{% include _template %}}'
+                f'{{% endmacro %}}',
+                template,
+                lambda: True
+            )
         # end of ckan changes
         pieces = loaders.split_template_path(template)
         for searchpath in searchpaths:
@@ -261,20 +273,61 @@ def make_call_node(*kw: Any) -> Any:
         return nodes.Output([make_call_node()]).set_lineno(tag.lineno)
 
 
-class SnippetExtension(BaseExtension):
+class SnippetExtension(ext.Extension):
     ''' Custom snippet tag
 
     {% snippet <template_name> [, <fallback_template_name>]...
                [, <keyword>=<value>]... %}
 
-    see lib.helpers.snippet() for more details.
+    renders <template_name> with <keyword>=<value> as local variables
     '''
 
-    tags = set(['snippet'])
+    tags = {'snippet'}
+
+    def parse(self, parser: Parser):
+        lineno = next(parser.stream).lineno
+        templates: list[nodes.Expr] = []
+        targets: list[nodes.Name] = []
+        kwargs: list[nodes.Keyword] = []
+        while not parser.stream.current.test_any('block_end'):
+            if templates or targets:
+                parser.stream.expect('comma')
+            if parser.stream.current.test('name') and parser.stream.look().test('assign'):
+                key = parser.parse_assign_target()
+                key.set_ctx('param')
+                targets.append(key)
+                parser.stream.expect('assign')
+                value = parser.parse_expression()
+                kwargs.append(nodes.Keyword(key.name, value, lineno=lineno))
+            else:
+                templates.append(parser.parse_expression())
+
+        imp = nodes.FromImport(lineno=lineno)
+        args = '§' + ','.join(targ.name for targ in targets)
+        template = 'dynamic' + args + '.html'
+        if isinstance(templates[0], nodes.Const):
+            template = args.join(path.splitext(templates[0].value))
+        imp.template = nodes.Const(template)
+        imp.names = ['snippet']
+        imp.with_context = False
+        nam = nodes.Name('snippet', 'load', lineno=lineno)
+        call = nodes.Call(
+            nam,
+            [nodes.List(templates, lineno=lineno)],
+            kwargs,
+            None,
+            None,
+            lineno=lineno
+        )
+        call.node = nam
+        out = nodes.Output(lineno=lineno)
+        out.nodes = [call]
+        wit = nodes.With(lineno=lineno)
+        wit.targets = []
+        wit.values = []
+        wit.body = [imp, out]
+        return wit
 
-    @classmethod
-    def _call(cls, args: Iterable[Any], kwargs: dict[str, Any]):
-        return base.render_snippet(*args, **kwargs)
 
 class UrlForStaticExtension(BaseExtension):
     ''' Custom url_for_static tag for getting a path for static assets.
diff --git a/ckan/templates/group/read.html b/ckan/templates/group/read.html
index a8022a1fefd..573b4af6739 100644
--- a/ckan/templates/group/read.html
+++ b/ckan/templates/group/read.html
@@ -21,7 +21,7 @@
   {% endblock %}
   {% block packages_list %}
     {% if page.items %}
-      {{ h.snippet('snippets/package_list.html', packages=page.items) }}
+      {% snippet 'snippets/package_list.html', packages=page.items %}
     {% endif %}
   {% endblock %}
   {% block page_pagination %}
@@ -34,7 +34,7 @@
   <div class="filters">
     <div>
       {% for facet in facet_titles %}
-        {{ h.snippet('snippets/facet_list.html', title=facet_titles[facet], name=facet, extras={'id':group_dict.id}, search_facets=search_facets) }}
+        {% snippet 'snippets/facet_list.html', title=facet_titles[facet], name=facet, extras={'id':group_dict.id}, search_facets=search_facets %}
       {% endfor %}
     </div>
     <a class="close no-text hide-filters"><i class="fa fa-times-circle"></i><span class="text">close</span></a>
diff --git a/ckan/templates/organization/bulk_process.html b/ckan/templates/organization/bulk_process.html
index 68f00f39a6c..23073108cc8 100644
--- a/ckan/templates/organization/bulk_process.html
+++ b/ckan/templates/organization/bulk_process.html
@@ -107,8 +107,8 @@ <h3 class="dataset-heading">
 {% endblock %}
 
 {% block secondary_content %}
-    {{  super() }}
-    {% for facet in c.facet_titles %}
-    {{ h.snippet('snippets/facet_list.html', title=c.facet_titles[facet], name=facet, extras={'id':group_dict.id}, search_facets=search_facets) }}
+  {{ super() }}
+  {% for facet in c.facet_titles %}
+    {% snippet 'snippets/facet_list.html', title=c.facet_titles[facet], name=facet, extras={'id':group_dict.id}, search_facets=search_facets %}
   {% endfor %}
 {% endblock %}
diff --git a/ckan/templates/organization/read.html b/ckan/templates/organization/read.html
index 466002aeaea..5ae33b3b77a 100644
--- a/ckan/templates/organization/read.html
+++ b/ckan/templates/organization/read.html
@@ -27,7 +27,7 @@
     {% endblock %}
   {% block packages_list %}
     {% if page.items %}
-      {{ h.snippet('snippets/package_list.html', packages=page.items) }}
+      {% snippet 'snippets/package_list.html', packages=page.items %}
     {% endif %}
   {% endblock %}
   {% block page_pagination %}
@@ -39,7 +39,7 @@
   <div class="filters">
     <div>
       {% for facet in facet_titles %}
-        {{ h.snippet('snippets/facet_list.html', title=facet_titles[facet], name=facet, extras={'id':group_dict.id}, search_facets=search_facets) }}
+        {% snippet 'snippets/facet_list.html', title=facet_titles[facet], name=facet, extras={'id':group_dict.id}, search_facets=search_facets %}
       {% endfor %}
     </div>
     <a class="close no-text hide-filters"><i class="fa fa-times-circle"></i><span class="text">close</span></a>
diff --git a/ckan/templates/package/search.html b/ckan/templates/package/search.html
index 7eb5452e204..971fca8f056 100644
--- a/ckan/templates/package/search.html
+++ b/ckan/templates/package/search.html
@@ -13,7 +13,7 @@
       {% block page_primary_action %}
         {% if h.check_access('package_create') %}
           <div class="page_primary_action">
-            {{ h.snippet ('snippets/add_dataset.html', dataset_type=dataset_type) }}
+            {% snippet 'snippets/add_dataset.html', dataset_type=dataset_type %}
           </div>
         {% endif %}
       {% endblock %}
@@ -35,7 +35,7 @@
         {% snippet 'snippets/search_form.html', form_id='dataset-search-form', type=dataset_type, query=q, sorting=sorting, sorting_selected=sort_by_selected, count=page.item_count, placeholder=h.humanize_entity_type('package', dataset_type, 'search placeholder') or _('Search datasets...'), facets=facets, show_empty=request.args, error=query_error, fields=fields %}
       {% endblock %}
       {% block package_search_results_list %}
-        {{ h.snippet('snippets/package_list.html', packages=page.items) }}
+        {% snippet 'snippets/package_list.html', packages=page.items %}
       {% endblock %}
     </div>
 
@@ -64,7 +64,7 @@
   <div class="filters">
     <div>
       {% for facet in facet_titles %}
-        {{ h.snippet('snippets/facet_list.html', title=facet_titles[facet], name=facet, search_facets=search_facets) }}
+        {% snippet 'snippets/facet_list.html', title=facet_titles[facet], name=facet, search_facets=search_facets %}
       {% endfor %}
     </div>
     <a class="close no-text hide-filters"><i class="fa fa-times-circle"></i><span class="text">close</span></a>
diff --git a/ckan/templates/package/snippets/package_form.html b/ckan/templates/package/snippets/package_form.html
index 67c32d2fba9..4bf3539937a 100644
--- a/ckan/templates/package/snippets/package_form.html
+++ b/ckan/templates/package/snippets/package_form.html
@@ -6,7 +6,7 @@
 <form id="dataset-edit" method="post" action="{{ action }}" data-module="basic-form" novalidate>
   {{ h.csrf_input() }}
   {% block stages %}
-    {{ h.snippet('package/snippets/stages.html', stages=stage, dataset_type=dataset_type) }}
+    {% snippet 'package/snippets/stages.html', stages=stage, dataset_type=dataset_type %}
   {% endblock %}
 
   <input type="hidden" name="_ckan_phase" value="dataset_new_1" />
diff --git a/ckan/templates/package/snippets/resource_form.html b/ckan/templates/package/snippets/resource_form.html
index f90892a70fa..494459430d2 100644
--- a/ckan/templates/package/snippets/resource_form.html
+++ b/ckan/templates/package/snippets/resource_form.html
@@ -10,7 +10,7 @@
   {% block stages %}
     {# An empty stages variable will not show the stages #}
     {% if stage %}
-      {{ h.snippet('package/snippets/stages.html', stages=stage, pkg_name=pkg_name, dataset_type=dataset_type) }}
+      {% snippet 'package/snippets/stages.html', stages=stage, pkg_name=pkg_name, dataset_type=dataset_type %}
     {% endif %}
   {% endblock %}
 


Test patch: