From 1f543f6eb368c3b23aa1f9cd2d23f0dba1456dec Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 30 Jan 2017 12:44:15 +0200 Subject: Add notion of load phase generation --- build2/context | 73 +++++++++++++++++++++++++++----- build2/context.cxx | 7 ++- build2/parser.cxx | 2 +- build2/target.cxx | 2 +- build2/types | 33 --------------- build2/variable | 10 +++-- build2/variable.cxx | 12 ++++-- unit-tests/test/script/parser/driver.cxx | 2 +- 8 files changed, 86 insertions(+), 55 deletions(-) diff --git a/build2/context b/build2/context index cf5483e..2f0a76f 100644 --- a/build2/context +++ b/build2/context @@ -29,7 +29,7 @@ namespace build2 // The build system model (internal state) is protected at the top level by // the model mutex. During serial execution the model mutex is unlocked. // - extern shared_mutex model; + extern shared_mutex model_mutex; // Parallel execution always starts with acquiring a shared model lock (by // creating model_slock; see below). Pointers to these locks are cached in @@ -38,14 +38,18 @@ namespace build2 // The build system starts with a "serial load" phase and then continues // with parallel search & match and execute. Search & match, however, can be // interrupted with an "exclusive load" by re-locking the shared lock as - // exclusive, changing the phase, and loading additional buildfiles. + // exclusive (using model_rlock below), changing the phase, and loading + // additional buildfiles. // // Serial load can perform arbitrary changes to the model. Exclusive load, - // however, can only perform "pure appends". That is, it can create new - // "nodes" (variables, scopes, etc) but not change already existing nodes - // or invalidate any references to such (the idea here is that one should - // be able to load additional buildfiles as long as they don't interfere - // with the existing build state). + // however, can only perform "island appends". That is, it can create new + // "nodes" (variables, scopes, etc) but not change already existing nodes or + // invalidate any references to such (the idea here is that one should be + // able to load additional buildfiles as long as they don't interfere with + // the existing build state). The "islands" are identified by the + // load_generation number (0 for serial load). It is incremented/restored by + // phase_guard and is stored in various "nodes" (variables, etc) to allow + // modifications "within the islands". // // @@ MT: do we really have to hold shared lock during execute? // @@ MT: we can also interrupt load s&m with execute -- neither handled @@ -59,10 +63,27 @@ namespace build2 #endif slock* model_lock; + extern size_t load_generation; + struct phase_guard { - explicit phase_guard (run_phase p): o (phase) {phase = p;} - ~phase_guard () {phase = o;} + explicit + phase_guard (run_phase p) + : o (phase) + { + phase = p; + + if (phase == run_phase::load) + ++load_generation; + } + + ~phase_guard () + { + if (phase == run_phase::load) + --load_generation; + + phase = o; + } run_phase o; }; @@ -124,7 +145,7 @@ namespace build2 if (slock* l = model_lock) assert (l->owns_lock ()); else - model_lock = &(l_ = slock (model)); + model_lock = &(l_ = slock (model_mutex)); } ~model_slock () @@ -140,6 +161,38 @@ namespace build2 slock l_; }; + // Re-lock shared to exclusive for the lifetime or rlock. + // + struct model_rlock + { + model_rlock () + : sl_ (model_lock) + { + if (sl_ != nullptr) + { + sl_->unlock (); + ul_ = ulock (*sl_->mutex ()); + } + } + + ~model_rlock () + { + if (sl_ != nullptr) + { + ul_.unlock (); + sl_->lock (); + } + } + + // Can be treated as const ulock. + // + operator const ulock& () const {return ul_;} + + private: + slock* sl_; + ulock ul_; + }; + // Cached variables. // extern const variable* var_src_root; diff --git a/build2/context.cxx b/build2/context.cxx index a8fe2df..a326002 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -25,7 +25,7 @@ namespace build2 { run_phase phase = run_phase::load; - shared_mutex model; + shared_mutex model_mutex; #ifdef __cpp_thread_local thread_local @@ -34,6 +34,8 @@ namespace build2 #endif slock* model_lock; + size_t load_generation; + const variable* var_src_root; const variable* var_out_root; const variable* var_src_base; @@ -188,7 +190,8 @@ namespace build2 // Add it if not found. // if (o->override == nullptr) - o->override.reset (new variable {n + k, nullptr, nullptr, v}); + const_cast (o)->override.reset ( + new variable {n + k, nullptr, nullptr, v, 0}); o = o->override.get (); diff --git a/build2/parser.cxx b/build2/parser.cxx index fe12b98..af6a6f5 100644 --- a/build2/parser.cxx +++ b/build2/parser.cxx @@ -1593,7 +1593,7 @@ namespace build2 if (type != nullptr) { if (var.type == nullptr) - var.type = type; + var_pool.update (const_cast (var), type); else if (var.type != type) fail (l) << "changing variable " << var << " type from " << var.type->name << " to " << type->name; diff --git a/build2/target.cxx b/build2/target.cxx index 24bd246..f04e26f 100644 --- a/build2/target.cxx +++ b/build2/target.cxx @@ -630,7 +630,7 @@ namespace build2 { // Relock for exclusive access and change to the load phase. // - rlock rl (model_lock); + model_rlock rl; phase_guard pg (run_phase::load); pair sp (switch_scope (*s.root_scope (), out_base)); diff --git a/build2/types b/build2/types index 1e7a347..daf10d3 100644 --- a/build2/types +++ b/build2/types @@ -86,39 +86,6 @@ namespace build2 using slock = std::shared_lock; using ulock = std::unique_lock; - // Re-lock shared to exclusive for the lifetime or rlock. - // - struct rlock - { - explicit - rlock (slock* sl) - : sl_ (sl) - { - if (sl_ != nullptr) - { - sl_->unlock (); - ul_ = ulock (*sl_->mutex ()); - } - } - - ~rlock () - { - if (sl_ != nullptr) - { - ul_.unlock (); - sl_->lock (); - } - } - - // Can be treated as const ulock. - // - operator const ulock& () const {return ul_;} - - private: - slock* sl_; - ulock ul_; - }; - // Exceptions. // // While is included, there is no using for std::exception -- diff --git a/build2/variable b/build2/variable index 876ae2b..e60c7a1 100644 --- a/build2/variable +++ b/build2/variable @@ -116,9 +116,10 @@ namespace build2 struct variable { string name; - mutable const value_type* type; // If NULL, then not (yet) typed. - mutable unique_ptr override; + const value_type* type; // If NULL, then not (yet) typed. + unique_ptr override; variable_visibility visibility; + size_t generation; // load_generation of this variable. }; inline bool @@ -912,11 +913,12 @@ namespace build2 void update (variable&, const build2::value_type*, - const variable_visibility*, - const bool*) const; + const variable_visibility* = nullptr, + const bool* = nullptr) const; // Entities that can access bypassing the lock. // + friend class parser; friend class scope; friend variable_overrides reset (const strings&); diff --git a/build2/variable.cxx b/build2/variable.cxx index 8695623..16fbe74 100644 --- a/build2/variable.cxx +++ b/build2/variable.cxx @@ -922,9 +922,12 @@ namespace build2 bool uv (v != nullptr && var.visibility != *v); // In the global pool existing variables can only be updated during - // serial load. + // the same load generation or during serial execution. // - assert (!global_ || !(ut || uv) || model_lock == nullptr); + assert (!global_ || + !(ut || uv) || + load_generation == 0 || + var.generation == load_generation); // Update type? // @@ -1012,7 +1015,8 @@ namespace build2 move (n), t, nullptr, - v != nullptr ? *v : variable_visibility::normal})); + v != nullptr ? *v : variable_visibility::normal, + load_generation})); variable& r (p.first->second); @@ -1034,6 +1038,8 @@ namespace build2 bool o, variable_visibility v) { + assert (!global_ || phase == run_phase::load); + size_t pn (p.size ()); size_t w (p.find ('*')); diff --git a/unit-tests/test/script/parser/driver.cxx b/unit-tests/test/script/parser/driver.cxx index f1e7483..c054669 100644 --- a/unit-tests/test/script/parser/driver.cxx +++ b/unit-tests/test/script/parser/driver.cxx @@ -182,7 +182,7 @@ namespace build2 // be absolute. However, the testscript implementation doesn't // really care. // - ulock ml (model); + ulock ml (model_mutex); file& tt ( targets.insert (work, -- cgit v1.1