From 219e00f3b8caec38a9c8fbb4d70e33455aba5a92 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Thu, 13 Oct 2022 09:00:11 +0200
Subject: Optimize by going straight to public variable pool where applicable

---
 build2/cli/init.cxx           |  4 +++-
 libbuild2/bin/init.cxx        | 27 +++++++++++++++++-----
 libbuild2/c/init.cxx          |  5 ++++-
 libbuild2/cc/init.cxx         |  5 ++++-
 libbuild2/cc/module.cxx       |  5 ++++-
 libbuild2/config/init.cxx     |  7 ++++--
 libbuild2/config/utility.cxx  |  4 ++--
 libbuild2/config/utility.hxx  |  4 ++--
 libbuild2/cxx/init.cxx        |  9 +++++---
 libbuild2/dist/init.cxx       | 52 ++++++++++++++++++++++++-------------------
 libbuild2/file.cxx            |  4 +++-
 libbuild2/in/init.cxx         |  5 ++++-
 libbuild2/install/init.cxx    | 27 ++++++++++++----------
 libbuild2/install/utility.hxx |  2 +-
 libbuild2/module.cxx          |  4 ++--
 libbuild2/test/init.cxx       | 10 ++++++---
 16 files changed, 112 insertions(+), 62 deletions(-)

diff --git a/build2/cli/init.cxx b/build2/cli/init.cxx
index a00fd7f..d7d8251 100644
--- a/build2/cli/init.cxx
+++ b/build2/cli/init.cxx
@@ -72,7 +72,9 @@ namespace build2
 
       // Enter metadata variables.
       //
-      auto& vp (rs.var_pool ());
+      // They are all qualified so go straight for the public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       auto& v_ver (vp.insert<string> ("cli.version"));
       auto& v_sum (vp.insert<string> ("cli.checksum"));
diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx
index 265521e..4b6090a 100644
--- a/libbuild2/bin/init.cxx
+++ b/libbuild2/bin/init.cxx
@@ -54,11 +54,14 @@ namespace build2
 
       // Enter variables.
       //
+      // All the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
+
       // Target is a string and not target_triplet because it can be
       // specified by the user.
       //
-      auto& vp (rs.var_pool ());
-
       vp.insert<string>    ("config.bin.target");
       vp.insert<string>    ("config.bin.pattern");
 
@@ -628,7 +631,10 @@ namespace build2
       //
       if (first)
       {
-        auto& vp (rs.var_pool ());
+        // All the variables we enter are qualified so go straight for the
+        // public variable pool.
+        //
+        auto& vp (rs.var_pool (true /* public */));
 
         vp.insert<path> ("config.bin.ar");
         vp.insert<path> ("config.bin.ranlib");
@@ -802,7 +808,10 @@ namespace build2
       //
       if (first)
       {
-        auto& vp (rs.var_pool ());
+        // All the variables we enter are qualified so go straight for the
+        // public variable pool.
+        //
+        auto& vp (rs.var_pool (true /* public */));
 
         vp.insert<path> ("config.bin.ld");
       }
@@ -960,7 +969,10 @@ namespace build2
       //
       if (first)
       {
-        auto& vp (rs.var_pool ());
+        // All the variables we enter are qualified so go straight for the
+        // public variable pool.
+        //
+        auto& vp (rs.var_pool (true /* public */));
 
         vp.insert<path> ("config.bin.rc");
       }
@@ -1059,7 +1071,10 @@ namespace build2
       //
       if (first)
       {
-        auto& vp (rs.var_pool ());
+        // All the variables we enter are qualified so go straight for the
+        // public variable pool.
+        //
+        auto& vp (rs.var_pool (true /* public */));
 
         vp.insert<path> ("config.bin.nm");
       }
diff --git a/libbuild2/c/init.cxx b/libbuild2/c/init.cxx
index 4c051b0..0b6c85b 100644
--- a/libbuild2/c/init.cxx
+++ b/libbuild2/c/init.cxx
@@ -154,7 +154,10 @@ namespace build2
 
       // Enter all the variables and initialize the module data.
       //
-      auto& vp (rs.var_pool ());
+      // All the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       cc::config_data d {
         cc::lang::c,
diff --git a/libbuild2/cc/init.cxx b/libbuild2/cc/init.cxx
index b2f35aa..33a1133 100644
--- a/libbuild2/cc/init.cxx
+++ b/libbuild2/cc/init.cxx
@@ -86,7 +86,10 @@ namespace build2
 
       // Enter variables.
       //
-      auto& vp (rs.var_pool ());
+      // All the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       auto v_t (variable_visibility::target);
 
diff --git a/libbuild2/cc/module.cxx b/libbuild2/cc/module.cxx
index 27f9d9e..6583001 100644
--- a/libbuild2/cc/module.cxx
+++ b/libbuild2/cc/module.cxx
@@ -41,7 +41,10 @@ namespace build2
 
       config::save_module (rs, x, 250);
 
-      auto& vp (rs.var_pool ());
+      // All the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       // Must already exist.
       //
diff --git a/libbuild2/config/init.cxx b/libbuild2/config/init.cxx
index 72ded05..7035ce5 100644
--- a/libbuild2/config/init.cxx
+++ b/libbuild2/config/init.cxx
@@ -100,7 +100,10 @@ namespace build2
       // reserved to not be valid module names (`build`). We also currently
       // treat `import` as special.
       //
-      auto& vp (rs.var_pool ());
+      // All the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       // NOTE: all config.** variables are by default made (via a pattern) to
       // be overridable with global visibility. So we must override this if a
@@ -234,7 +237,7 @@ namespace build2
                  ? &extra.module_as<module> ()
                  : nullptr);
 
-      auto& vp (rs.var_pool ());
+      auto& vp (rs.var_pool (true /* public */));
 
       // Note: config.* is pattern-typed to global visibility.
       //
diff --git a/libbuild2/config/utility.cxx b/libbuild2/config/utility.cxx
index 314de84..6574367 100644
--- a/libbuild2/config/utility.cxx
+++ b/libbuild2/config/utility.cxx
@@ -130,7 +130,7 @@ namespace build2
       // Pattern-typed as bool.
       //
       const variable& var (
-        rs.var_pool ().insert ("config." + n + ".configured"));
+        rs.var_pool (true).insert ("config." + n + ".configured"));
 
       save_variable (rs, var);
 
@@ -144,7 +144,7 @@ namespace build2
       // Pattern-typed as bool.
       //
       const variable& var (
-        rs.var_pool ().insert ("config." + n + ".configured"));
+        rs.var_pool (true).insert ("config." + n + ".configured"));
 
       save_variable (rs, var);
 
diff --git a/libbuild2/config/utility.hxx b/libbuild2/config/utility.hxx
index 98d7ec0..1e2ff53 100644
--- a/libbuild2/config/utility.hxx
+++ b/libbuild2/config/utility.hxx
@@ -435,7 +435,7 @@ namespace build2
       const V* cv (
         cast_null<V> (
           lookup_config (rs,
-                         rs.var_pool ().insert<V> ("config." + var),
+                         rs.var_pool (true).insert<V> ("config." + var),
                          std::forward<T> (default_value)))); // VC14
 
       value& v (bs.assign<V> (move (var)));
@@ -453,7 +453,7 @@ namespace build2
       const V* cv (
         cast_null<V> (
           lookup_config (rs,
-                         rs.var_pool ().insert<V> ("config." + var),
+                         rs.var_pool (true).insert<V> ("config." + var),
                          std::forward<T> (default_value)))); // VC14
 
       value& v (bs.append<V> (move (var)));
diff --git a/libbuild2/cxx/init.cxx b/libbuild2/cxx/init.cxx
index 3a934db..5396056 100644
--- a/libbuild2/cxx/init.cxx
+++ b/libbuild2/cxx/init.cxx
@@ -93,7 +93,7 @@ namespace build2
 
       // Feature flags.
       //
-      auto& vp (rs.var_pool ());
+      auto& vp (rs.var_pool (true /* public */)); // All qualified.
 
       // Similar to config.cxx.std, config.cxx.features.* overrides
       // cxx.features.*.
@@ -469,7 +469,10 @@ namespace build2
 
       // Enter all the variables and initialize the module data.
       //
-      auto& vp (rs.var_pool ());
+      // All the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       cc::config_data d {
         cc::lang::cxx,
@@ -811,7 +814,7 @@ namespace build2
       auto& cm (
         load_module<config_module> (rs, rs, "cxx.config", loc, extra.hints));
 
-      auto& vp (rs.var_pool ());
+      auto& vp (rs.var_pool (true /* public */)); // All qualified.
 
       bool modules (cast<bool> (rs["cxx.features.modules"]));
 
diff --git a/libbuild2/dist/init.cxx b/libbuild2/dist/init.cxx
index 5e371bf..26ff86d 100644
--- a/libbuild2/dist/init.cxx
+++ b/libbuild2/dist/init.cxx
@@ -33,7 +33,34 @@ namespace build2
       // Enter module variables. Do it during boot in case they get assigned
       // in bootstrap.build (which is customary for, e.g., dist.package).
       //
-      auto& vp (rs.var_pool ());
+
+      // The dist flag or path. Normally it is a flag (true or false) but can
+      // also be used to remap the distribution location.
+      //
+      // In the latter case it specifies the "imaginary" source location which
+      // is used to derive the corresponding distribution local. This location
+      // can be specified as either a directory path (to remap with the same
+      // file name) or a file path (to remap with a different name). And the
+      // way we distinguish between the two is via the presence/absence of the
+      // trailing directory separator. If the path is relative, then it's
+      // treated relative to the target directory. Note that to make things
+      // less error prone, simple paths without any directory separators are
+      // not allowed (use ./<name> instead).
+      //
+      // Note that if multiple targets end up with the same source location,
+      // the behavior is undefined and no diagnostics is issued.
+      //
+      // Note also that such remapping has no effect in the bootstrap
+      // distribution mode.
+      //
+      // Note: project-private.
+      //
+      rs.var_pool ().insert<path> ("dist", variable_visibility::target);
+
+      // The rest of the variables we enter are qualified so go straight for
+      // the public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       // config.dist.archives is a list of archive extensions (e.g., zip,
       // tar.gz) that can be optionally prefixed with a directory. If it is
@@ -72,27 +99,6 @@ namespace build2
       vp.insert<paths>        ("dist.archives");
       vp.insert<paths>        ("dist.checksums");
 
-      // The dist flag or path. Normally it is a flag (true or false) but can
-      // also be used to remap the distribution location.
-      //
-      // In the latter case it specifies the "imaginary" source location which
-      // is used to derive the corresponding distribution local. This location
-      // can be specified as either a directory path (to remap with the same
-      // file name) or a file path (to remap with a different name). And the
-      // way we distinguish between the two is via the presence/absence of the
-      // trailing directory separator. If the path is relative, then it's
-      // treated relative to the target directory. Note that to make things
-      // less error prone, simple paths without any directory separators are
-      // not allowed (use ./<name> instead).
-      //
-      // Note that if multiple targets end up with the same source location,
-      // the behavior is undefined and no diagnostics is issued.
-      //
-      // Note also that such remapping has no effect in the bootstrap
-      // distribution mode.
-      //
-      vp.insert<path> ("dist", variable_visibility::target);
-
       // Project's package name. Note: if set, must be in bootstrap.build.
       //
       auto& v_d_p (vp.insert<string> ("dist.package"));
@@ -202,7 +208,7 @@ namespace build2
 
       l5 ([&]{trace << "for " << rs;});
 
-      auto& vp (rs.var_pool ());
+      auto& vp (rs.var_pool (true /* public */)); // All qualified.
 
       // Register our wildcard rule. Do it explicitly for the alias to prevent
       // something like insert<target>(dist_id, test_id) taking precedence.
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index cb57f70..6cd4402 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -2138,7 +2138,9 @@ namespace build2
     // over anything that we may discover. In particular, we will prefer it
     // over any bundled subprojects.
     //
-    auto& vp (iroot.var_pool ());
+    // Note: go straight for the public variable pool.
+    //
+    auto& vp (iroot.var_pool (true /* public */));
 
     using config::lookup_config;
 
diff --git a/libbuild2/in/init.cxx b/libbuild2/in/init.cxx
index 8bd5909..2fb73e1 100644
--- a/libbuild2/in/init.cxx
+++ b/libbuild2/in/init.cxx
@@ -34,7 +34,10 @@ namespace build2
       // Enter variables.
       //
       {
-        auto& vp (rs.var_pool ());
+        // All the variables we enter are qualified so go straight for the
+        // public variable pool.
+        //
+        auto& vp (rs.var_pool (true /* public */));
 
         // Alternative variable substitution symbol with '$' being the
         // default.
diff --git a/libbuild2/install/init.cxx b/libbuild2/install/init.cxx
index 35c2d13..1a6ee0f 100644
--- a/libbuild2/install/init.cxx
+++ b/libbuild2/install/init.cxx
@@ -166,6 +166,8 @@ namespace build2
 
       bool global (*name == '\0');
 
+      auto& vp (rs.var_pool (true /* default */)); // All qualified.
+
       if (spec)
       {
         vn = "config.install";
@@ -175,7 +177,7 @@ namespace build2
           vn += name;
         }
         vn += var;
-        const variable& vr (rs.var_pool ().insert<CT> (move (vn)));
+        const variable& vr (vp.insert<CT> (move (vn)));
 
         using config::lookup_config;
 
@@ -192,7 +194,7 @@ namespace build2
       vn = "install.";
       vn += name;
       vn += var;
-      const variable& vr (rs.var_pool ().insert<T> (move (vn)));
+      const variable& vr (vp.insert<T> (move (vn)));
 
       value& v (rs.assign (vr));
 
@@ -236,7 +238,7 @@ namespace build2
       // This one doesn't have config.* value (only set in a buildfile).
       //
       if (!global)
-        rs.var_pool ().insert<bool> (string ("install.") + n + ".subdirs");
+        rs.var_pool (true).insert<bool> (string ("install.") + n + ".subdirs");
     }
 
     void
@@ -252,8 +254,6 @@ namespace build2
 
       // Enter module variables (note that init() below enters some more).
       //
-      auto& vp (rs.var_pool ());
-
       // The install variable is a path, not dir_path, since it can be used
       // to both specify the target directory (to install with the same file
       // name) or target file (to install with a different name). And the
@@ -263,7 +263,8 @@ namespace build2
       // Plus it can have the special true/false values when acting as a
       // operation variable.
       //
-      auto& ovar (vp.insert<path> ("install", variable_visibility::target));
+      auto& ovar (rs.var_pool ().insert<path> ("install",
+                                               variable_visibility::target));
 
       // Register the install function family if this is the first instance of
       // the install modules.
@@ -327,15 +328,17 @@ namespace build2
 
       // Enter module variables.
       //
-      auto& vp (rs.var_pool ());
+      rs.var_pool ().insert<bool> ("for_install", variable_visibility::prereq);
+
+      // The rest of the variables we enter are qualified so go straight
+      // for the public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
 
       // Note that the set_dir() calls below enter some more.
       //
-      {
-        vp.insert<bool>   ("for_install", variable_visibility::prereq);
-        vp.insert<string> ("install.mode");
-        vp.insert<bool>   ("install.subdirs");
-      }
+      vp.insert<string> ("install.mode");
+      vp.insert<bool>   ("install.subdirs");
 
       // Environment.
       //
diff --git a/libbuild2/install/utility.hxx b/libbuild2/install/utility.hxx
index 52b9a54..3e2dcad 100644
--- a/libbuild2/install/utility.hxx
+++ b/libbuild2/install/utility.hxx
@@ -43,7 +43,7 @@ namespace build2
     {
       auto r (
         s.target_vars[tt]["*"].insert (
-          *s.var_pool ().find ("install.mode")));
+          *s.ctx.var_pool.find ("install.mode")));
 
       if (r.second) // Already set by the user?
         r.first = move (m);
diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx
index b7b9bbb..f200a63 100644
--- a/libbuild2/module.cxx
+++ b/libbuild2/module.cxx
@@ -674,7 +674,7 @@ namespace build2
       i->boot_init = e.init;
     }
 
-    rs.assign (rs.var_pool ().insert (mod + ".booted")) = (mf != nullptr);
+    rs.assign (rs.var_pool (true).insert (mod + ".booted")) = (mf != nullptr);
   }
 
   void
@@ -743,7 +743,7 @@ namespace build2
     // buildfile-visible (where we use the term "load a module"; see the note
     // on terminology above)
     //
-    auto& vp (rs.var_pool ());
+    auto& vp (rs.var_pool (true));
     value& lv (bs.assign (vp.insert (mod + ".loaded")));
     value& cv (bs.assign (vp.insert (mod + ".configured")));
 
diff --git a/libbuild2/test/init.cxx b/libbuild2/test/init.cxx
index f62ddcc..b7cf25f 100644
--- a/libbuild2/test/init.cxx
+++ b/libbuild2/test/init.cxx
@@ -33,7 +33,11 @@ namespace build2
       // Enter module variables. Do it during boot in case they get assigned
       // in bootstrap.build.
       //
-      auto& vp (rs.var_pool ());
+      // Most of the variables we enter are qualified so go straight for the
+      // public variable pool.
+      //
+      auto& vp (rs.var_pool (true /* public */));
+      auto& pvp (rs.var_pool ()); // For `test` and `for_test`.
 
       common_data d {
 
@@ -64,7 +68,7 @@ namespace build2
         // The test variable is a name which can be a path (with the
         // true/false special values) or a target name.
         //
-        vp.insert<name>    ("test", variable_visibility::target),
+        pvp.insert<name>   ("test", variable_visibility::target),
         vp.insert<strings> ("test.options"),
         vp.insert<strings> ("test.arguments"),
 
@@ -106,7 +110,7 @@ namespace build2
 
       // This one is used by other modules/rules.
       //
-      vp.insert<bool> ("for_test", variable_visibility::prereq);
+      pvp.insert<bool> ("for_test", variable_visibility::prereq);
 
       // These are only used in testscript.
       //
-- 
cgit v1.1