diff options
Diffstat (limited to 'libbuild2')
| -rw-r--r-- | libbuild2/context.cxx | 34 | ||||
| -rw-r--r-- | libbuild2/context.hxx | 10 | ||||
| -rw-r--r-- | libbuild2/target.cxx | 25 | ||||
| -rw-r--r-- | libbuild2/target.hxx | 36 |
4 files changed, 84 insertions, 21 deletions
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx index 8052716..7944294 100644 --- a/libbuild2/context.cxx +++ b/libbuild2/context.cxx @@ -848,7 +848,7 @@ namespace build2 } } - bool run_phase_mutex:: + optional<bool> run_phase_mutex:: relock (run_phase o, run_phase n) { // Pretty much a fused unlock/lock implementation except that we always @@ -857,6 +857,7 @@ namespace build2 assert (o != n); bool r; + bool s (true); // True switch. if (o == run_phase::load) lm_.unlock (); @@ -919,6 +920,13 @@ namespace build2 { if (!lm_.try_lock ()) { + // If we failed to acquire the load mutex, then we know there is (or + // was) someone before us in the load phase. And it's impossible to + // switch to a different phase between our calls to try_lock() above + // and lock() below because of our +1 in lc_. + // + s = false; + ctx_.sched.deactivate (false /* external */); lm_.lock (); ctx_.sched.activate (false /* external */); @@ -928,7 +936,7 @@ namespace build2 r = !fail_; // Re-query. } - return r; + return r ? optional<bool> (s) : nullopt; } // C++17 deprecated uncaught_exception() so use uncaught_exceptions() if @@ -1043,7 +1051,8 @@ namespace build2 phase_lock* pl (phase_lock_instance); assert (&pl->ctx == &ctx); - if (!ctx.phase_mutex.relock (old_phase, new_phase)) + optional<bool> r (ctx.phase_mutex.relock (old_phase, new_phase)); + if (!r) { ctx.phase_mutex.relock (new_phase, old_phase); throw failed (); @@ -1052,14 +1061,31 @@ namespace build2 pl->phase = new_phase; if (new_phase == run_phase::load) // Note: load lock is exclusive. + { ctx.load_generation++; + // Invalidate cached target base_scope values if we are switching from a + // non-load phase (we don't cache during load which means load->load + // switch doesn't have anything to invalidate). + // + // @@ This is still quite expensive on project like Boost with a large + // number of files (targets) and a large number of load phase + // switches (due to directory buildfiles). + // + if (*r) + { + for (const unique_ptr<target>& t: ctx.targets) + t->base_scope_.store (nullptr, memory_order_relaxed); + } + } + //text << this_thread::get_id () << " phase switch " // << old_phase << " " << new_phase; } #if 0 - // NOTE: see push/pop_phase() logic if trying to enable this. + // NOTE: see push/pop_phase() logic if trying to enable this. Also + // the load stuff above. // phase_switch:: phase_switch (phase_unlock&& u, phase_lock&& l) diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx index fbdf9b8..5cc2115 100644 --- a/libbuild2/context.hxx +++ b/libbuild2/context.hxx @@ -28,6 +28,7 @@ namespace build2 public: // Acquire a phase lock potentially blocking (unless already in the // desired phase) until switching to the desired phase is possible. + // Return false on failure. // bool lock (run_phase); @@ -38,9 +39,14 @@ namespace build2 void unlock (run_phase); - // Switch from one phase to another. + // Switch from one phase to another. Return nullopt on failure (so can be + // used as bool), true if switched from a different phase, and false if + // joined/switched to the same phase (this, for example, can be used to + // decide if a phase switching housekeeping is really necessary). Note: + // currently only implemented for the load phase (always returns true + // for the others). // - bool + optional<bool> relock (run_phase unlock, run_phase lock); // Statistics. diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index 92db7e9..1806e61 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -115,22 +115,27 @@ namespace build2 } const scope& target:: - base_scope () const + base_scope_impl () const { // If this target is from the src tree, use its out directory to find // the scope. // - return ctx.scopes.find_out (out_dir ()); - } + const scope& s (ctx.scopes.find_out (out_dir ())); - const scope& target:: - root_scope () const - { - // This is tricky to cache so we do the lookup for now. + // Cache unless we are in the load phase. // - const scope* r (base_scope ().root_scope ()); - assert (r != nullptr); - return *r; + if (ctx.phase != run_phase::load) + { + const scope* e (nullptr); + if (!base_scope_.compare_exchange_strong ( + e, + &s, + memory_order_release, + memory_order_consume)) + assert (e == &s); + } + + return s; } pair<lookup, size_t> target:: diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 3d8c4ff..d3e88dd 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -380,7 +380,16 @@ namespace build2 // Most qualified scope that contains this target. // const scope& - base_scope () const; + base_scope () const + { + if (ctx.phase != run_phase::load) + { + if (const scope* s = base_scope_.load (memory_order_consume)) + return *s; + } + + return base_scope_impl (); + } // Root scope of a project that contains this target. Note that // a target can be out of any (known) project root in which case @@ -388,7 +397,10 @@ namespace build2 // then use base_scope().root_scope() expression instead. // const scope& - root_scope () const; + root_scope () const + { + return *base_scope ().root_scope (); + } // Root scope of a bundle amalgamation that contains this target. The // same notes as to root_scope() apply. @@ -414,6 +426,16 @@ namespace build2 return out_dir ().sub (s.out_path ()); } + // Implementation details (use above functions instead). + // + // Base scope cached value. Invalidated every time we switch to the load + // phase (which is the only phase where we may insert new scopes). + // + mutable atomic<const scope*> base_scope_ {nullptr}; + + const scope& + base_scope_impl () const; + // Prerequisites. // // We use an atomic-empty semantics that allows one to "swap in" a set of @@ -1580,10 +1602,14 @@ namespace build2 // Note: not MT-safe so can only be used during serial execution. // public: - using iterator = butl::map_iterator_adapter<map_type::const_iterator>; + using iterator = butl::map_iterator_adapter<map_type::iterator>; + using const_iterator = butl::map_iterator_adapter<map_type::const_iterator>; + + iterator begin () {return map_.begin ();} + iterator end () {return map_.end ();} - iterator begin () const {return map_.begin ();} - iterator end () const {return map_.end ();} + const_iterator begin () const {return map_.begin ();} + const_iterator end () const {return map_.end ();} size_t size () const {return map_.size ();} |