From 45c9008b9679aeed32487f065e3e594f320c8b9f Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 2 Jul 2020 08:41:02 +0200 Subject: Optimize variable extraction in bootstrap_src() --- libbuild2/config/init.cxx | 7 ++- libbuild2/file.cxx | 122 ++++++++++++++++++++++------------------------ libbuild2/file.hxx | 19 +++----- 3 files changed, 69 insertions(+), 79 deletions(-) diff --git a/libbuild2/config/init.cxx b/libbuild2/config/init.cxx index aa2c763..69da09c 100644 --- a/libbuild2/config/init.cxx +++ b/libbuild2/config/init.cxx @@ -204,13 +204,12 @@ namespace build2 // Assume missing version is 0. // - auto p (extract_variable (rs.ctx, lex, c_v)); - uint64_t v (p.second ? cast (p.first) : 0); + optional ov (extract_variable (rs.ctx, lex, c_v)); + uint64_t v (ov ? cast (*ov) : 0); if (v != module::version) fail (l) << "incompatible config file " << in << - info << "config file version " << v - << (p.second ? "" : " (missing)") << + info << "config file version " << v << (ov ? "" : " (missing)") << info << "config module version " << module::version << info << "consider reconfiguring " << project (rs) << '@' << rs.out_path (); diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 4431bd1..a5ff070 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -476,17 +476,17 @@ namespace build2 // We cannot just source the buildfile since there is no scope to do // this on yet. // - auto p (extract_variable (ctx, f, *ctx.var_out_root)); - - if (!p.second) - fail << "variable out_root expected as first line in " << f; - - auto r (convert (move (p.first))); + if (optional v = extract_variable (ctx, f, *ctx.var_out_root)) + { + auto r (convert (move (*v))); - if (r.relative ()) - fail << "relative path in out_root value in " << f; + if (r.relative ()) + fail << "relative path in out_root value in " << f; - return r; + return r; + } + else + fail << "variable out_root expected as first line in " << f << endf; } static void @@ -572,8 +572,8 @@ namespace build2 return v; } - pair - extract_variable (context& ctx, lexer& l, const variable& var, size_t n) + optional + extract_variable (context& ctx, lexer& l, const variable& var) { const path_name& fn (l.name ()); @@ -581,27 +581,13 @@ namespace build2 { token t (l.next ()); - // Skip the requested number of lines. - // - while (--n != 0) - { - for (; t.type != token_type::eos; t = l.next ()) - { - if (t.type == token_type::newline) - { - t = l.next (); - break; - } - } - } - token_type tt; if (t.type != token_type::word || t.value != var.name || ((tt = l.next ().type) != token_type::assign && tt != token_type::prepend && tt != token_type::append)) { - return make_pair (value (), false); + return nullopt; } parser p (ctx); @@ -613,7 +599,7 @@ namespace build2 // Steal the value, the scope is going away. // - return make_pair (move (*v), true); + return move (*v); } catch (const io_error& e) { @@ -621,25 +607,23 @@ namespace build2 } } - pair + optional extract_variable (context& ctx, istream& is, const path& bf, - const variable& var, size_t n) + const variable& var) { path_name in (bf); lexer l (is, in); - return extract_variable (ctx, l, var, n); + return extract_variable (ctx, l, var); } - pair - extract_variable (context& ctx, - const path& bf, - const variable& var, size_t n) + optional + extract_variable (context& ctx, const path& bf, const variable& var) { try { ifdstream ifs (bf); - return extract_variable (ctx, ifs, bf, var, n); + return extract_variable (ctx, ifs, bf, var); } catch (const io_error& e) { @@ -710,15 +694,15 @@ namespace build2 } else { - auto p (extract_variable (ctx, f, *ctx.var_src_root)); + optional v (extract_variable (ctx, f, *ctx.var_src_root)); - if (!p.second) + if (!v) fail << "variable src_root expected as first line in " << f; - if (cast (p.first).relative ()) + if (cast (*v).relative ()) fail << "relative path in src_root value in " << f; - src_root_v = move (p.first); + src_root_v = move (*v); remap_src_root (ctx, src_root_v); // Remap if inside old_src_root. src_root = &cast (src_root_v); @@ -735,13 +719,13 @@ namespace build2 if (f.empty ()) fail << "no build/bootstrap.build in " << *src_root; - auto p (extract_variable (ctx, f, *ctx.var_project)); - - if (!p.second) + if (optional v = extract_variable (ctx, f, *ctx.var_project)) + { + name = cast (move (*v)); + } + else fail << "variable " << *ctx.var_project << " expected as a first " << "line in " << f; - - name = cast (move (p.first)); } l5 ([&]{trace << "extracted project name '" << name << "' for " @@ -868,31 +852,41 @@ namespace build2 // else if (rs.buildfiles.insert (bf).second) { - // Extract and pre-cache the project name before loading - // bootstrap.build. + // Extract the project name and amalgamation variable value so that + // we can make them available while loading bootstrap.build. // - // @@ TODO: optimize by extracting both of them at once? + // In case of amalgamation, we only deal with the empty variable value + // (which indicates that amalgamating this project is disabled). We go + // through all this trouble of extracting its value manually (and thus + // requiring its assignment, if any, to be the second line in + // bootstrap.build, after project assignment) in order to have the + // logical amalgamation view during bootstrap (note that the bootstrap + // pre hooks will still see physical amalgamation). // - auto pp (extract_variable (ctx, bf, *ctx.var_project, 1)); + optional pv, av; + try + { + ifdstream ifs (bf); + path_name bfn (bf); + lexer l (ifs, bfn); - if (!pp.second) - fail << "variable " << *ctx.var_project << " expected as a first " - << "line in " << bf; + pv = extract_variable (ctx, l, *ctx.var_project); - const project_name pn (cast (move (pp.first))); - rs.root_extra->project = &pn; + if (!pv) + fail << "variable " << *ctx.var_project << " expected as a first " + << "line in " << bf; - // Deal with the empty amalgamation variable (which indicates that - // amalgamating this project is disabled). We go through all this - // trouble of extracting its value manually (and thus requiring its - // assignment, if any, to be the second line in bootstrap.build, after - // project assignment) in order to have the logical amalgamation view - // during bootstrap (note that the bootstrap pre hooks will still see - // physical amalgamation). - // - auto ap (extract_variable (ctx, bf, *ctx.var_amalgamation, 2)); + av = extract_variable (ctx, l, *ctx.var_amalgamation); + } + catch (const io_error& e) + { + fail << "unable to read buildfile " << bf << ": " << e; + } + + const project_name pn (cast (move (*pv))); + rs.root_extra->project = &pn; - if (ap.second && (ap.first.null || ap.first.empty ())) + if (av && (av->null || av->empty ())) rs.root_extra->amalgamation = nullptr; { @@ -907,7 +901,7 @@ namespace build2 // Detect and diagnose the case where the amalgamation variable is not // the second line. // - if (!ap.second && rs.vars[ctx.var_amalgamation].defined ()) + if (!av && rs.vars[ctx.var_amalgamation].defined ()) { fail << "variable " << *ctx.var_amalgamation << " expected as a " << "second line in " << bf; diff --git a/libbuild2/file.hxx b/libbuild2/file.hxx index 3bceb80..f64f130 100644 --- a/libbuild2/file.hxx +++ b/libbuild2/file.hxx @@ -210,27 +210,24 @@ namespace build2 load_root (scope& root); // Extract the specified variable value from a buildfile. It is expected to - // be the n'th non-blank/comment line and not to rely on any variable + // be the first non-blank/comment line and not to rely on any variable // expansions other than those from the global scope or any variable - // overrides. Return in second an indication of whether the variable was - // found. + // overrides. Return nullopt if the variable was not found. // - LIBBUILD2_SYMEXPORT pair - extract_variable (context&, const path&, const variable&, size_t n = 1); + LIBBUILD2_SYMEXPORT optional + extract_variable (context&, const path&, const variable&); // As above, but extract from a stream. The path argument is used for // diagnostics. // - LIBBUILD2_SYMEXPORT pair - extract_variable (context&, - istream&, const path&, - const variable&, size_t = 1); + LIBBUILD2_SYMEXPORT optional + extract_variable (context&, istream&, const path&, const variable&); // As above, but extract from a lexer (this could be useful for extracting // from stdin). // - LIBBUILD2_SYMEXPORT pair - extract_variable (context&, lexer&, const variable&, size_t = 1); + LIBBUILD2_SYMEXPORT optional + extract_variable (context&, lexer&, const variable&); // Import has two phases: the first is triggered by the import directive in // the buildfile. It will try to find and load the project. Failed that, it -- cgit v1.1