From ea2f77171f5ee3b704e4acdf925c989c070c563f Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 15 May 2017 13:59:08 +0200 Subject: Update all prerequisites before extracting header dependencies --- build2/cc/compile.cxx | 225 ++++++++++++++++++++++++++++---------------------- build2/cc/compile.hxx | 4 +- build2/depdb.hxx | 9 +- 3 files changed, 133 insertions(+), 105 deletions(-) diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx index b8b362d..a05c712 100644 --- a/build2/cc/compile.cxx +++ b/build2/cc/compile.cxx @@ -38,7 +38,7 @@ namespace build2 struct match_data { prerequisite_member src; - timestamp dd_mtime; // depdb mtime, timestamp_nonexistent if outdated. + timestamp mt; }; static_assert (sizeof (match_data) <= target::data_size, @@ -71,7 +71,7 @@ namespace build2 { // Save in the target's auxilary storage. // - t.data (match_data {p, timestamp_nonexistent}); + t.data (match_data {p, timestamp_unknown}); return true; } } @@ -220,6 +220,69 @@ namespace build2 } } + // Update the target during the match phase. Return true if it has changed + // or if the passed timestamp is not timestamp_unknown and is older than + // the target. + // + // This function is used to make sure header dependencies are up to date. + // + // There would normally be a lot of headers for every source file (think + // all the system headers) and just calling execute_direct() on all of + // them can get expensive. At the same time, most of these headers are + // existing files that we will never be updating (again, system headers, + // for example) and the rule that will match them is the fallback + // file_rule. That rule has an optimization: it returns noop_recipe (which + // causes the target state to be automatically set to unchanged) if the + // file is known to be up to date. So we do the update "smartly". + // + static bool + update (tracer& trace, action act, const target& t, timestamp ts) + { + const path_target* pt (t.is_a ()); + + if (pt == nullptr) + ts = timestamp_unknown; + + target_state os (t.matched_state (act)); + + if (os == target_state::unchanged) + { + if (ts == timestamp_unknown) + return false; + else + { + // We expect the timestamp to be known (i.e., existing file). + // + timestamp mt (pt->mtime ()); // @@ MT perf: know target state. + assert (mt != timestamp_unknown); + return mt > ts; + } + } + else + { + // We only want to return true if our call to execute() actually + // caused an update. In particular, the target could already have been + // in target_state::changed because of a dependency extraction run for + // some other source file. + // + // @@ MT perf: so we are going to switch the phase and execute for + // any generated header. + // + phase_switch ps (run_phase::execute); + target_state ns (execute_direct (act, t)); + + if (ns != os && ns != target_state::unchanged) + { + l6 ([&]{trace << "updated " << t + << "; old state " << os + << "; new state " << ns;}); + return true; + } + else + return ts != timestamp_unknown ? pt->newer (ts) : false; + } + } + recipe compile:: apply (action act, target& xt) const { @@ -362,7 +425,6 @@ namespace build2 ? unmatch::safe : unmatch::none)) pt = nullptr; // Ignore in execute. - } // Inject additional prerequisites. We only do it when performing update @@ -457,13 +519,50 @@ namespace build2 l4 ([&]{trace << "source file mismatch forcing update of " << t;}); // If any of the above checks resulted in a mismatch (different - // compiler, options, or source file), or if the database is newer - // than the target (interrupted update) then force the target update. + // compiler, options, or source file) or if the depdb is newer than + // the target, then do unconditional update. + // + timestamp mt; + bool u (dd.writing () || dd.mtime () > (mt = file_mtime (tp))); + + // Update prerequisite targets (normally just the source file). // - md.dd_mtime = dd.writing () ? timestamp_nonexistent : dd.mtime (); + // This is an unusual place and time to do it. But we have to do it + // before extracting dependencies. The reasoning for source file is + // pretty clear. What other prerequisites could we have? While + // normally they will be some other sources (as in, static content + // from project's src_root), its possible they are some auto-generated + // stuff. And it's possible they affect the preprocessor result. Say + // some ad hoc/out-of-band compiler input file that is passed via the + // command line. So, to be safe, we make everything is up to date. + // + for (const target* pt: pts) + { + if (pt == nullptr || pt == dir) + continue; + + if (update (trace, act, *pt, u ? timestamp_unknown : mt)) + { + // If the file got updated or is newer than the database, then we + // cannot rely on the cache any further. However, the cached data + // could actually still be valid so the compiler run in inject() + // will validate it. + // + // We do need to update the database timestamp, however. Failed + // that, we will keep re-validating the cached data over and over + // again. + // + if (dd.reading ()) + dd.touch (); - inject (act, t, lo, src, dd); + u = true; + } + } + + u = inject (act, t, lo, src, dd) || u; dd.close (); + + md.mt = u ? timestamp_nonexistent : mt; } switch (act) @@ -794,7 +893,7 @@ namespace build2 } } - void compile:: + bool compile:: inject (action act, target& t, lorder lo, const file& src, depdb& dd) const { tracer trace (x, "compile::inject"); @@ -932,68 +1031,14 @@ namespace build2 // date then we use the same restart and skip logic to switch to the // compiler run. // - - // Update the target "smartly". Return true if it has changed or if the - // passed timestamp is not timestamp_unknown and is older than the - // target. - // - // There would normally be a lot of headers for every source file (think - // all the system headers) and just calling execute_direct() on all of - // them can get expensive. At the same time, most of these headers are - // existing files that we will never be updating (again, system headers, - // for example) and the rule that will match them is the fallback - // file_rule. That rule has an optimization: it returns noop_recipe - // (which causes the target state to be automatically set to unchanged) - // if the file is known to be up to date. - // - auto update = [&trace, act] (const path_target& pt, timestamp ts) -> bool - { - target_state os (pt.matched_state (act)); - - if (os == target_state::unchanged) - { - if (ts == timestamp_unknown) - return false; - else - { - // We expect the timestamp to be known (i.e., existing file). - // - timestamp mt (pt.mtime ()); // @@ MT perf: know target state. - assert (mt != timestamp_unknown); - return mt > ts; - } - } - else - { - // We only want to restart if our call to execute() actually caused - // an update. In particular, the target could already have been in - // target_state::changed because of a dependency extraction run for - // some other source file. - // - // @@ MT perf: so we are going to switch the phase and execute for - // any generated header. - // - phase_switch ps (run_phase::execute); - target_state ns (execute_direct (act, pt)); - - if (ns != os && ns != target_state::unchanged) - { - l6 ([&]{trace << "updated " << pt - << "; old state " << os - << "; new state " << ns;}); - return true; - } - else - return ts != timestamp_unknown ? pt.newer (ts) : false; - } - }; + bool u (false); // Updated flag (result). // Update and add a header file to the list of prerequisite targets. // Depending on the cache flag, the file is assumed to either have come // from the depdb cache or from the compiler run. Return whether the // extraction process should be restarted. // - auto add = [&trace, &update, &pm, act, &t, lo, &dd, &bs, this] + auto add = [&trace, &pm, act, &t, lo, &dd, &bs, &u, this] (path f, bool cache) -> bool { // Find or maybe insert the target. @@ -1171,7 +1216,11 @@ namespace build2 // the db itself (if it has changed since the db was written, then // chances are the cached data is stale). // - bool restart (update (*pt, cache ? dd.mtime () : timestamp_unknown)); + bool restart ( + update ( + trace, act, *pt, cache ? dd.mtime () : timestamp_unknown)); + + u = u || restart; // Verify/add it to the dependency database. We do it after update in // order not to add bogus files (non-existent and without a way to @@ -1187,29 +1236,10 @@ namespace build2 return restart; }; - // If nothing so far has invalidated the dependency database, then - // try the cached data before running the compiler. - // - bool cache (dd.reading ()); - - // But, before we do all that, make sure the source file itself if up to - // date. + // If nothing so far has invalidated the dependency database, then try + // the cached data before running the compiler. // - if (update (src, dd.mtime ())) - { - // If the file got updated or is newer than the database, then we - // cannot rely on the cache any further. However, the cached data - // could actually still be valid so the compiler run will validate it. - // - // We do need to update the database timestamp, however. Failed that, - // we will keep re-validating the cached data over and over again. - // - if (cache) - { - cache = false; - dd.touch (); - } - } + bool cache (dd.reading () && !dd.touched ()); size_t skip_count (0); for (bool restart (true); restart; cache = false) @@ -1468,6 +1498,8 @@ namespace build2 } } } + + return u; } // Filter cl.exe noise (msvc.cxx). @@ -1482,23 +1514,16 @@ namespace build2 const path& tp (t.path ()); const match_data& md (t.data ()); - // Update prerequisites and determine if any relevant ones render us - // out-of-date. Note that currently we treat all the prerequisites - // as potentially affecting the result (for simplicity/performance). - // - timestamp mt; - - // If the depdb was overwritten or it's newer than the target, then - // do unconditional update. + // While all our prerequisites are already up-to-date, we still have + // to execute them to keep the dependency counts straight. // - if (md.dd_mtime == timestamp_nonexistent || - md.dd_mtime > (mt = t.load_mtime ())) - mt = timestamp_nonexistent; - - auto pr (execute_prerequisites (x_src, act, t, mt)); + auto pr (execute_prerequisites (x_src, act, t, md.mt)); if (pr.first) + { + t.mtime (md.mt); return *pr.first; + } const file& s (pr.second); diff --git a/build2/cc/compile.hxx b/build2/cc/compile.hxx index bee13f2..47b966e 100644 --- a/build2/cc/compile.hxx +++ b/build2/cc/compile.hxx @@ -80,9 +80,9 @@ namespace build2 const target_type* map_extension (const scope&, const string&, const string&) const; - // Header dependency injection. + // Header dependency injection. Return true if any were updated. // - void + bool inject (action, target&, lorder, const file&, depdb&) const; private: diff --git a/build2/depdb.hxx b/build2/depdb.hxx index 9dffa28..b747096 100644 --- a/build2/depdb.hxx +++ b/build2/depdb.hxx @@ -106,13 +106,16 @@ namespace build2 // won't be because of eof. // bool - more () {return state_ == state::read;} + more () const {return state_ == state::read;} bool - reading () {return state_ != state::write;} + reading () const {return state_ != state::write;} bool - writing () {return state_ == state::write;} + writing () const {return state_ == state::write;} + + bool + touched () const {return touch_;} // Write the next line. Note that this switches the database into the // write mode and no further reading will be possible. -- cgit v1.1