From a8ef1b4fd1bb9321bd909258256002c062e27f98 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike.gerwitz@ryansg.com>
Date: Thu, 12 Oct 2023 12:58:47 -0400
Subject: [PATCH] Begin to use symtable-map for template/macro passes

I wanted to get this committed before I continue because it required changes
to the `expand-sequence` system---tunneling params cannot pass through
functions, so this accepts a context to pass back to the calling system via
the `eseq:expand-node` override.

Otherwise, the key change here is the elimination of a preproc:symtable
XPath within a `template/@match`, which was a huge performance problem with
the preceding commits.  This improves build times modestly, but there are
more changes that this sets up for, so I'll keep going.

DEV-15095
---
 src/current/include/preproc/package.xsl  | 24 +++++++-
 src/current/include/preproc/template.xsl | 70 ++++++++++++------------
 src/preproc/expand/expand-sequence.xsl   | 25 ++++++---
 3 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/src/current/include/preproc/package.xsl b/src/current/include/preproc/package.xsl
index 0426cf33..ddbe22ee 100644
--- a/src/current/include/preproc/package.xsl
+++ b/src/current/include/preproc/package.xsl
@@ -46,6 +46,25 @@
 
 <include href="../../compiler/fragments.xsl" />
 
+<!--
+  Generate a symbol table map from the provided package.
+
+  This uses XSLT maps, which are backed by a hash table in Saxon.  Note
+  that, while this does provide constant-time lookup (vs. a linear scan of
+  the entire symbol table), there is a cost associated with its
+  construction, so you must be sure that the savings afforded by the lookups
+  makes up for that cost.
+-->
+<function name="preproc:mk-symtable-map"
+          as="map( xs:string, element( preproc:sym ) )">
+  <!-- in practice this is lv:package or lv:rater -->
+  <param name="pkg" as="element()" />
+
+  <sequence select="map:merge(
+                      for $sym in $pkg/preproc:symtable/preproc:sym
+                        return map{ string( $sym/@name ) : $sym } )" />
+</function>
+
 
 <!-- begin preprocessing from an arbitrary node -->
 <template name="preproc:pkg-compile" as="element( lv:package )"
@@ -63,7 +82,10 @@
 
   <!-- macro expansion -->
   <variable name="stage1" as="element()+">
-    <apply-templates select="." mode="preproc:macropass" />
+    <apply-templates select="." mode="preproc:macropass">
+      <with-param name="symtable-map" tunnel="yes"
+                  select="preproc:mk-symtable-map( . )" />
+    </apply-templates>
 
     <message>
       <text>[preproc] *macro pass complete; expanding...</text>
diff --git a/src/current/include/preproc/template.xsl b/src/current/include/preproc/template.xsl
index c327a7e8..5211aea5 100644
--- a/src/current/include/preproc/template.xsl
+++ b/src/current/include/preproc/template.xsl
@@ -23,6 +23,7 @@
 <stylesheet version="2.0"
             xmlns="http://www.w3.org/1999/XSL/Transform"
             xmlns:xs="http://www.w3.org/2001/XMLSchema"
+            xmlns:map="http://www.w3.org/2005/xpath-functions/map"
             xmlns:preproc="http://www.lovullo.com/rater/preproc"
             xmlns:lv="http://www.lovullo.com/rater"
             xmlns:t="http://www.lovullo.com/rater/apply-template"
@@ -105,6 +106,8 @@
   TODO: This causes an extra pass; we'd like to avoid having to do that.
 -->
 <template match="t:*" mode="preproc:macros" priority="5">
+  <param name="symtable-map" as="map(*)" tunnel="yes" />
+
   <!-- TODO: debug flag
   <message>
     <text>[preproc] expanding template shorthand for </text>
@@ -137,7 +140,7 @@
 
   <!-- XXX: a large chunk of this is duplicate code; factor out -->
   <variable name="tpl" as="element( lv:template )?"
-                select="preproc:locate-template( $name, root( . ) )" />
+                select="preproc:locate-template( $symtable-map, $name, root( . ) )" />
 
   <variable name="src-root" as="element( lv:package )"
                 select="if ( root(.)/lv:package ) then
@@ -161,12 +164,12 @@
 
 <function name="preproc:locate-template"
               as="element( lv:template )?">
+  <param name="symtable-map" as="map(*)" />
   <param name="name" as="xs:string" />
   <param name="root" as="element( lv:package )" />
 
   <variable name="sym" as="element( preproc:sym )?"
-                select="$root/preproc:symtable/preproc:sym[
-                          @name = $name ]" />
+                select="$symtable-map( $name )" />
 
   <variable name="package" as="element( lv:package )?"
                 select="if ( $sym/@src ) then
@@ -202,29 +205,23 @@
   Note that if the attribute shorthand is used for params, one extra expansion
   will have to occur, which is an additional performance hit.
 -->
-<template match="lv:apply-template[
-                       @name=root(.)/preproc:symtable/preproc:sym/@name ]
-                     |lv:apply-template[ @name=root(.)//lv:template/@name ]"
-              mode="preproc:macros" priority="6">
+<template mode="preproc:macros" priority="6"
+          match="lv:apply-template">
+  <param name="symtable-map" as="map(*)" tunnel="yes" />
 
   <variable name="name" select="@name" />
   <variable name="attrparams" select="@*[ not( local-name() = 'name' ) ]" />
 
   <!-- used for type checking -->
-  <variable name="root" as="element( lv:package )"
+  <variable name="src-root" as="element( lv:package )"
                 select="if ( root( . ) instance of document-node() ) then
                           root( . )/lv:package
                         else
                           root( . )" />
 
-  <variable name="src-root" as="element( lv:package )"
-                select="if ( $root/lv:package ) then
-                          $root/lv:package
-                        else
-                          $root" />
-
   <variable name="tpl" as="element( lv:template )?"
-                select="preproc:locate-template( $name, $root )" />
+                select="preproc:locate-template(
+                          $symtable-map, $name, $src-root )" />
 
   <choose>
     <when test="exists( $tpl ) and $attrparams">
@@ -254,7 +251,16 @@
     </when>
 
     <otherwise>
-      <preproc:error>Undefined template <value-of select="$name" /></preproc:error>
+      <!-- keep this application around for later -->
+      <sequence select="." />
+
+      <!-- nothing we can do yet -->
+      <message>
+        <text>[preproc] deferring application of unknown template </text>
+        <value-of select="@name" />
+      </message>
+
+      <preproc:repass need-sym="{@name}" />
     </otherwise>
   </choose>
 </template>
@@ -546,24 +552,6 @@
 </template>
 
 
-<!--
-  This block is used when we attempt to apply a template that has not been
-  defined
--->
-<template match="lv:apply-template" mode="preproc:macros" priority="5">
-  <!-- keep this application around for later -->
-  <sequence select="." />
-
-  <!-- nothing we can do yet -->
-  <message>
-    <text>[preproc] deferring application of unknown template </text>
-    <value-of select="@name" />
-  </message>
-
-  <preproc:repass need-sym="{@name}" />
-</template>
-
-
 <template match="lv:template/lv:param" mode="preproc:apply-template" priority="5">
   <param name="first-child" select="false()" />
 
@@ -1512,7 +1500,11 @@
 -->
 <template mode="preproc:macros" priority="5"
               match="lv:expand-sequence">
-  <sequence select="eseq:expand-step( . )" />
+  <param name="symtable-map" as="map(*)" tunnel="yes" />
+
+  <!-- tunnels do not continue through functions, so store the symtable-map
+       as a context that will be passed back to us by eseq:expand-node -->
+  <sequence select="eseq:expand-step( $symtable-map, . )" />
 </template>
 
 
@@ -1570,10 +1562,16 @@
 
 
 <function name="eseq:expand-node" as="node()*">
+  <!-- this is the symtable-map, as passed in via eseq:expand-step -->
+  <param name="context" as="map(*)" />
+
   <param name="node" as="node()" />
 
   <apply-templates mode="preproc:macros"
-                       select="$node" />
+                       select="$node">
+    <with-param name="symtable-map" tunnel="yes"
+                select="$context" />
+  </apply-templates>
 </function>
 
 
diff --git a/src/preproc/expand/expand-sequence.xsl b/src/preproc/expand/expand-sequence.xsl
index e04163f9..8caccdfd 100644
--- a/src/preproc/expand/expand-sequence.xsl
+++ b/src/preproc/expand/expand-sequence.xsl
@@ -38,7 +38,7 @@
   generally unnecessary.
 
   Expansion sequences are initiated by invoking
-  @ttref{eseq:expand-step#1} on any arbitrary node containing any number
+  @ttref{eseq:expand-step#2} on any arbitrary node containing any number
   of children to be expanded in order.  Each call will proceed one
   step (detailed herein), eventually resulting in each node expanded
   and the expansion sequence node eliminated.
@@ -58,6 +58,7 @@
   return an empty sequence in such a case.
 -->
 <function name="eseq:expand-step" as="node()*">
+  <param name="context" />
   <param name="eseq" as="node()*" />
 
   <variable name="count" as="xs:integer"
@@ -73,7 +74,9 @@
   <sequence select="$eseq[ position() lt $count ]" />
 
   <apply-templates mode="_eseq:expand"
-                   select="$target" />
+                   select="$target">
+    <with-param name="context" select="$context" />
+  </apply-templates>
 </function>
 
 
@@ -187,9 +190,11 @@
 -->
 <template mode="_eseq:expand" as="node()+"
           match="*[ node() ]">
+  <param name="context" />
+
   <choose>
     <when test="node()[1][ eseq:is-expandable( . ) ]">
-      <sequence select="_eseq:expand-head( . )" />
+      <sequence select="_eseq:expand-head( $context, . )" />
     </when>
 
     <otherwise>
@@ -250,7 +255,7 @@
   after all expansions are complete and the expansion sequence node
   itself is eliminated (per the final match above), then the node that
   was last expanded and hoisted will be considered to be the expansion
-  sequence by @ttref{eseq:expand-step#1}.  This is true, but should not
+  sequence by @ttref{eseq:expand-step#2}.  This is true, but should not
   be a problem in practice: hoisting is intended to place nodes into
   context for the caller; it is expected that the caller will
   recognize when to invoke sequence expansion (likely on a pre-defined
@@ -286,9 +291,10 @@
   function.
 
   Actual expansion is left to
-  @ref{eseq:expand-node#1,,@code{eseq:expand-node#1}}.
+  @ref{eseq:expand-node#2,,@code{eseq:expand-node#2}}.
 -->
 <function name="_eseq:expand-head" as="element()">
+  <param name="context" />
   <param name="eseq" as="element()" />
 
   <variable name="head" as="node()"
@@ -300,7 +306,7 @@
   <for-each select="$eseq">
     <copy>
       <sequence select="$eseq/@*,
-                        eseq:expand-node( $head ),
+                        eseq:expand-node( $context, $head ),
                         $head/following-sibling::node()" />
     </copy>
   </for-each>
@@ -314,7 +320,7 @@
 
   Its default behavior is an important consideration: what if
   @ttref{eseq:is-expandable#1} is overridden but the implementation
-  forgets to override @ttref{eseq:expand-node#1}?  If the default
+  forgets to override @ttref{eseq:expand-node#2}?  If the default
   behavior were to simply echo back the node, it seems likely that we
   would never finish processing, since the very node that matched the
   predicate to begin with would remain unchanged.
@@ -341,6 +347,7 @@
   prevent infinite recursion/iteration.
 -->
 <function name="eseq:expand-node" as="node()*">
+  <param name="context" />
   <param name="node" as="node()" />
 
   <eseq:expand-error>
@@ -352,7 +359,7 @@
 
 
 <!--
-  The return type of @ttref{eseq:expand-node#1} produces an interesting
+  The return type of @ttref{eseq:expand-node#2} produces an interesting
   concept.  Consider what may happen after an expansion:
 
   @enumerate
@@ -383,7 +390,7 @@
   continue processing; the expansion may have yielded additional
   symbols that must be added to the symbol table, for example.  The
   process will be continued on the next call to
-  @ttref{eseq:expand-step#1}.
+  @ttref{eseq:expand-step#2}.
 -->