From b47a323f09f1f9ccece03ca4d84dbe7a47ff8177 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Tue, 22 Oct 2019 10:02:23 +0200
Subject: Move global mutex shards to context

---
 build2/b.cxx                          | 27 ++++++++++++---------------
 libbuild2/context.cxx                 |  2 ++
 libbuild2/context.hxx                 | 14 ++++++++++++++
 libbuild2/file.cxx                    |  8 ++++----
 libbuild2/function.test.cxx           |  8 ++++++--
 libbuild2/module.cxx                  |  5 +++--
 libbuild2/scope.cxx                   | 10 ++++++----
 libbuild2/test/script/parser.test.cxx |  8 ++++++--
 libbuild2/variable.cxx                | 14 +++++++-------
 libbuild2/variable.hxx                | 29 ++++++++++++-----------------
 libbuild2/variable.ixx                |  2 +-
 libbuild2/variable.txx                | 10 +++++++---
 tests/libbuild2/driver.cxx            |  7 +++++--
 13 files changed, 85 insertions(+), 59 deletions(-)

diff --git a/build2/b.cxx b/build2/b.cxx
index d6b364d..0183e0f 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -556,19 +556,15 @@ main (int argc, char* argv[])
         fail << "invalid --max-jobs|-J value";
     }
 
-     sched.startup (jobs,
-                    1,
-                    max_jobs,
-                    jobs * ops.queue_depth (),
-                    (ops.max_stack_specified ()
-                     ? optional<size_t> (ops.max_stack () * 1024)
-                     : nullopt));
-
-    // @@ CTX: should these be per-context?
-    //
-    variable_cache_mutex_shard_size = sched.shard_size ();
-    variable_cache_mutex_shard.reset (
-      new shared_mutex[variable_cache_mutex_shard_size]);
+    sched.startup (jobs,
+                   1,
+                   max_jobs,
+                   jobs * ops.queue_depth (),
+                   (ops.max_stack_specified ()
+                    ? optional<size_t> (ops.max_stack () * 1024)
+                    : nullopt));
+
+    global_mutex_shards mutex_shards (sched.shard_size ());
 
     // Trace some overall environment information.
     //
@@ -588,10 +584,11 @@ main (int argc, char* argv[])
     // below).
     //
     unique_ptr<context> ctx;
-    auto new_context = [&ctx, &sched, &cmd_vars]
+    auto new_context = [&ctx, &sched, &mutex_shards, &cmd_vars]
     {
       ctx = nullptr; // Free first.
       ctx.reset (new context (sched,
+                              mutex_shards,
                               ops.dry_run (),
                               !ops.serial_stop () /* keep_going */,
                               cmd_vars));
@@ -1425,7 +1422,7 @@ main (int argc, char* argv[])
                 continue;
 
               // If we have a directory, enter the scope, similar to how we do
-              // it in the context's reset().
+              // it in the context ctor.
               //
               scope& s (o.dir
                         ? sm.insert ((out_base / *o.dir).normalize ())->second
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index cc0eb0b..23942a8 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -56,12 +56,14 @@ namespace build2
 
   context::
   context (scheduler& s,
+           global_mutex_shards& ms,
            bool dr,
            bool kg,
            const strings& cmd_vars,
            optional<context*> mc)
       : data_ (new data (*this)),
         sched (s),
+        mutex_shards (ms),
         dry_run_option (dr),
         keep_going (kg),
         phase_mutex (*this),
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index d2e38e7..cc34803 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -99,6 +99,18 @@ namespace build2
     mutex lm_;
   };
 
+  class global_mutex_shards
+  {
+  public:
+    size_t                     variable_cache_size;
+    unique_ptr<shared_mutex[]> variable_cache;
+
+    explicit
+    global_mutex_shards (size_t vc)
+        : variable_cache_size (vc),
+          variable_cache (new shared_mutex[variable_cache_size]) {}
+  };
+
   // @@ CTX: document (backlinks, non-overlap etc). RW story.
   //
   // A context can be preempted to execute another context (we do this, for
@@ -122,6 +134,7 @@ namespace build2
 
   public:
     scheduler& sched;
+    global_mutex_shards& mutex_shards;
 
     // Dry run flag (see --dry-run|-n).
     //
@@ -412,6 +425,7 @@ namespace build2
     //
     explicit
     context (scheduler&,
+             global_mutex_shards&,
              bool dry_run = false,
              bool keep_going = true,
              const strings& cmd_vars = {},
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index f0215df..68827ec 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -1306,8 +1306,8 @@ namespace build2
       // config.import.<proj>
       //
       {
-        // Note: pattern-typed in context.cxx:reset() as an overridable
-        // variable of type abs_dir_path (path auto-completion).
+        // Note: pattern-typed in context ctor as an overridable variable of
+        // type abs_dir_path (path auto-completion).
         //
         const variable& var (vp.insert (n));
 
@@ -1340,8 +1340,8 @@ namespace build2
       {
         auto lookup = [&iroot, &vp, &loc] (string name) -> path
         {
-          // Note: pattern-typed in context.cxx:reset() as an overridable
-          // variable of type path.
+          // Note: pattern-typed in context ctor as an overridable variable of
+          // type path.
           //
           const variable& var (vp.insert (move (name)));
 
diff --git a/libbuild2/function.test.cxx b/libbuild2/function.test.cxx
index bd0be62..c4f7fa2 100644
--- a/libbuild2/function.test.cxx
+++ b/libbuild2/function.test.cxx
@@ -42,8 +42,12 @@ namespace build2
     //
     init_diag (1);
     init (nullptr, argv[0]);
-    scheduler sched (1);  // Serial execution.
-    context ctx (sched);
+
+    // Serial execution.
+    //
+    scheduler sched (1);
+    global_mutex_shards shards (1);
+    context ctx (sched, shards);
 
     auto& functions (ctx.functions);
 
diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx
index b5914cd..11e0b4f 100644
--- a/libbuild2/module.cxx
+++ b/libbuild2/module.cxx
@@ -156,6 +156,7 @@ namespace build2
         //
         ctx.module_context_storage->reset (
           new context (ctx.sched,
+                       ctx.mutex_shards,
                        false,                    /* dry_run */
                        ctx.keep_going,
                        ctx.global_var_overrides, /* cmd_vars */
@@ -488,8 +489,8 @@ namespace build2
       }
     }
 
-    // Note: pattern-typed in context.cxx:reset() as project-visibility
-    // variables of type bool.
+    // Note: pattern-typed in context ctor as project-visibility variables of
+    // type bool.
     //
     // We call the variable 'loaded' rather than 'inited' because it is
     // buildfile-visible (where we use the term "load a module"; see the note
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index d6251a4..d076fce 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -28,10 +28,10 @@ namespace build2
 
     // Process target type/pattern-specific prepend/append values.
     //
-    auto pre_app = [&var] (lookup& l,
-                           const scope* s,
-                           const target_type* tt, const string* tn,
-                           const target_type* gt, const string* gn)
+    auto pre_app = [&var, this] (lookup& l,
+                                 const scope* s,
+                                 const target_type* tt, const string* tn,
+                                 const target_type* gt, const string* gn)
     {
       const value& v (*l);
       assert ((v.extra == 1 || v.extra == 2) && v.type == nullptr);
@@ -51,6 +51,7 @@ namespace build2
       //
       pair<value&, ulock> entry (
         s->target_vars.cache.insert (
+          ctx,
           make_tuple (&v, tt, *tn),
           stem,
           static_cast<const variable_map::value_data&> (v).version,
@@ -436,6 +437,7 @@ namespace build2
 
     pair<value&, ulock> entry (
       cache.insert (
+        ctx,
         make_pair (&var, inner_vars),
         stem,
         0, // Overrides are immutable.
diff --git a/libbuild2/test/script/parser.test.cxx b/libbuild2/test/script/parser.test.cxx
index 56630fe..2eec791 100644
--- a/libbuild2/test/script/parser.test.cxx
+++ b/libbuild2/test/script/parser.test.cxx
@@ -155,8 +155,12 @@ namespace build2
         //
         init_diag (1);
         init (nullptr, argv[0]);
-        scheduler sched (1); // Serial execution.
-        context ctx (sched);
+
+        // Serial execution.
+        //
+        scheduler sched (1);
+        global_mutex_shards shards (1);
+        context ctx (sched, shards);
 
         bool scope (false);
         bool id (false);
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index 22ea69d..1fe9914 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -359,13 +359,16 @@ namespace build2
   }
 
   void
-  typify_atomic (value& v, const value_type& t, const variable* var)
+  typify_atomic (context& ctx,
+                 value& v,
+                 const value_type& t,
+                 const variable* var)
   {
     // Typification is kind of like caching so we reuse that mutex shard.
     //
     shared_mutex& m (
-      variable_cache_mutex_shard[
-        hash<value*> () (&v) % variable_cache_mutex_shard_size]);
+      ctx.mutex_shards.variable_cache[
+        hash<value*> () (&v) % ctx.mutex_shards.variable_cache_size]);
 
     // Note: v.type is rechecked by typify() under lock.
     //
@@ -1139,7 +1142,7 @@ namespace build2
           const bool* o) const
   {
     // Check overridability (all overrides, if any, should already have
-    // been entered (see context.cxx:reset()).
+    // been entered; see context ctor for details).
     //
     if (var.overrides != nullptr && (o == nullptr || !*o))
       fail << "variable " << var.name << " cannot be overridden";
@@ -1514,9 +1517,6 @@ namespace build2
     return lookup ();
   }
 
-  size_t variable_cache_mutex_shard_size;
-  unique_ptr<shared_mutex[]> variable_cache_mutex_shard;
-
   template struct LIBBUILD2_DEFEXPORT value_traits<strings>;
   template struct LIBBUILD2_DEFEXPORT value_traits<vector<name>>;
   template struct LIBBUILD2_DEFEXPORT value_traits<paths>;
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index 3cb3d7f..29b8200 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -187,8 +187,8 @@ namespace build2
   // that, variables set by the C++ code are by default non-overridable.
   //
   // Initial processing including entering of global overrides happens in
-  // reset() before any other variables. Project wide overrides are entered in
-  // main(). Overriding happens in scope::find_override().
+  // context ctor before any other variables. Project wide overrides are
+  // entered in main(). Overriding happens in scope::find_override().
   //
   // NULL type and normal visibility are the defaults and can be overridden by
   // "tighter" values.
@@ -254,7 +254,7 @@ namespace build2
   inline ostream&
   operator<< (ostream& os, const variable& v) {return os << v.name;}
 
-  //
+  // A value (of a variable, function argument, etc).
   //
   class LIBBUILD2_SYMEXPORT value
   {
@@ -460,7 +460,7 @@ namespace build2
   void typify        (value&, const value_type&, const variable*);
 
   LIBBUILD2_SYMEXPORT void
-  typify_atomic (value&, const value_type&, const variable*);
+  typify_atomic (context&, value&, const value_type&, const variable*);
 
   // Remove value type from the value reversing it to names. This is similar
   // to reverse() below except that it modifies the value itself.
@@ -1005,8 +1005,8 @@ namespace build2
   extern template struct LIBBUILD2_DECEXPORT
   value_traits<std::map<project_name, dir_path>>;
 
-  // Project-wide (as opposed to global) variable overrides. Returned by
-  // context.cxx:reset().
+  // Project-wide (as opposed to global) variable overrides (see context ctor
+  // for details).
   //
   struct variable_override
   {
@@ -1438,7 +1438,7 @@ namespace build2
     void
     clear () {m_.clear ();}
 
-    // Implementation details.
+    // Implementation details (only used for empty_variable_map).
     //
   public:
     explicit
@@ -1468,15 +1468,11 @@ namespace build2
   // the references remain valid).
   //
   // Note that since the cache can be modified on any lookup (including during
-  // the execute phase), it is protected by its own mutex shard (allocated in
-  // main()). This shard is also used for value typification (which is kind of
-  // like caching) during concurrent execution phases.
+  // the execute phase), it is protected by its own mutex shard (see
+  // global_mutex_shards in context). This shard is also used for value
+  // typification (which is kind of like caching) during concurrent execution
+  // phases.
   //
-  LIBBUILD2_SYMEXPORT extern size_t variable_cache_mutex_shard_size;
-
-  LIBBUILD2_SYMEXPORT extern unique_ptr<shared_mutex[]>
-  variable_cache_mutex_shard;
-
   template <typename K>
   class variable_cache
   {
@@ -1486,7 +1482,7 @@ namespace build2
     // then typify the cached value.
     //
     pair<value&, ulock>
-    insert (K, const lookup& stem, size_t version, const variable&);
+    insert (context&, K, const lookup& stem, size_t version, const variable&);
 
   private:
     struct entry_type
@@ -1565,7 +1561,6 @@ namespace build2
     context& ctx;
     map_type map_;
     bool global_;
-
   };
 
   class LIBBUILD2_SYMEXPORT variable_type_map
diff --git a/libbuild2/variable.ixx b/libbuild2/variable.ixx
index e5353ed..d341c26 100644
--- a/libbuild2/variable.ixx
+++ b/libbuild2/variable.ixx
@@ -772,7 +772,7 @@ namespace build2
     else
     {
       if (v.type.load (memory_order_acquire) != var.type)
-        build2::typify_atomic (const_cast<value_data&> (v), *var.type, &var);
+        typify_atomic (*ctx, const_cast<value_data&> (v), *var.type, &var);
     }
   }
 
diff --git a/libbuild2/variable.txx b/libbuild2/variable.txx
index 92b7169..ed25cb6 100644
--- a/libbuild2/variable.txx
+++ b/libbuild2/variable.txx
@@ -600,7 +600,11 @@ namespace build2
   //
   template <typename K>
   pair<value&, ulock> variable_cache<K>::
-  insert (K k, const lookup& stem, size_t ver, const variable& var)
+  insert (context& ctx,
+          K k,
+          const lookup& stem,
+          size_t ver,
+          const variable& var)
   {
     using value_data = variable_map::value_data;
 
@@ -610,8 +614,8 @@ namespace build2
                  : 0);
 
     shared_mutex& m (
-      variable_cache_mutex_shard[
-        hash<variable_cache*> () (this) % variable_cache_mutex_shard_size]);
+      ctx.mutex_shards.variable_cache[
+        hash<variable_cache*> () (this) % ctx.mutex_shards.variable_cache_size]);
 
     slock sl (m);
     ulock ul (m, defer_lock);
diff --git a/tests/libbuild2/driver.cxx b/tests/libbuild2/driver.cxx
index a14d145..26c00d6 100644
--- a/tests/libbuild2/driver.cxx
+++ b/tests/libbuild2/driver.cxx
@@ -34,8 +34,11 @@ main (int, char* argv[])
   in::build2_in_load ();
   bash::build2_bash_load ();
 
-  scheduler sched (1); // Serial execution.
-  context ctx (sched);
+  // Serial execution.
+  //
+  scheduler sched (1);
+  global_mutex_shards shards (1);
+  context ctx (sched, shards);
 
   return 0;
 }
-- 
cgit v1.1