aboutsummaryrefslogtreecommitdiff
path: root/libbuild2
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2022-04-12 11:39:55 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2022-04-13 10:55:55 +0200
commitdc8f0267c332e405a698545c98478756165c908e (patch)
treee012a02a183bb25ea116e693dbdb51983ca0a85f /libbuild2
parent7376287554e30aa0b74136bf6c16566f6bda80cd (diff)
Cache target base scope lookups
Diffstat (limited to 'libbuild2')
-rw-r--r--libbuild2/context.cxx34
-rw-r--r--libbuild2/context.hxx10
-rw-r--r--libbuild2/target.cxx25
-rw-r--r--libbuild2/target.hxx36
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 ();}