From 5607313a91e5ca0113b1f8b9acfd02c1fb105346 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 19 Jan 2017 12:45:04 +0200 Subject: Get rid of project_name_pool With small string optimizations this is most likely a hindrance rather that an optimization. --- build2/algorithm | 2 +- build2/algorithm.cxx | 2 +- build2/algorithm.ixx | 2 +- build2/cc/common | 2 +- build2/cc/compile.cxx | 2 +- build2/cc/link.cxx | 7 +++++-- build2/cc/pkgconfig.cxx | 8 ++++---- build2/cc/utility.cxx | 2 +- build2/context | 1 - build2/context.cxx | 2 -- build2/dist/rule.cxx | 2 +- build2/file.cxx | 16 +++++++-------- build2/install/rule.cxx | 2 +- build2/name | 22 ++++++++++---------- build2/name.cxx | 4 ++-- build2/name.ixx | 8 +------- build2/parser | 8 ++++---- build2/parser.cxx | 53 +++++++++++++++++++++++++++---------------------- build2/prerequisite | 28 ++++++++++++-------------- build2/prerequisite.cxx | 9 ++++++--- build2/target | 6 ++++-- build2/target.ixx | 2 +- 22 files changed, 96 insertions(+), 94 deletions(-) diff --git a/build2/algorithm b/build2/algorithm index 3dbef4c..9a91781 100644 --- a/build2/algorithm +++ b/build2/algorithm @@ -46,7 +46,7 @@ namespace build2 const string& name, const string* ext, scope*, - const string* proj = nullptr); + const optional& proj = nullopt); // As above but specify the target type as template argument. // diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx index 8874e96..78af195 100644 --- a/build2/algorithm.cxx +++ b/build2/algorithm.cxx @@ -25,7 +25,7 @@ namespace build2 // If this is a project-qualified prerequisite, then this is import's // business. // - if (pk.proj != nullptr) + if (pk.proj) return import (pk); if (target* t = pk.tk.type->search (pk)) diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx index 09d522f..84b0ee7 100644 --- a/build2/algorithm.ixx +++ b/build2/algorithm.ixx @@ -32,7 +32,7 @@ namespace build2 const string& name, const string* ext, scope* scope, - const string* proj) + const optional& proj) { return search ( prerequisite_key {proj, {&type, &dir, &out, &name, ext}, scope}); diff --git a/build2/cc/common b/build2/cc/common index 2c79fa6..f0476f9 100644 --- a/build2/cc/common +++ b/build2/cc/common @@ -229,7 +229,7 @@ namespace build2 bin::lib&, bin::liba*, bin::libs*, - const string*, + const optional&, const string&, const dir_path&, const dir_paths&) const; // pkgconfig.cxx diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx index 3318f21..23499ad 100644 --- a/build2/cc/compile.cxx +++ b/build2/cc/compile.cxx @@ -277,7 +277,7 @@ namespace build2 // we don't need to do match() in order to get options (if // any, they would be set by search_library()). // - if (p.proj () == nullptr || + if (!p.proj () || search_library ( sys_lib_dirs, usr_lib_dirs, p.prerequisite) == nullptr) { diff --git a/build2/cc/link.cxx b/build2/cc/link.cxx index a6b67d5..8010598 100644 --- a/build2/cc/link.cxx +++ b/build2/cc/link.cxx @@ -153,7 +153,7 @@ namespace build2 // Handle imported libraries. // - if (p.proj () != nullptr) + if (p.proj ()) pt = search_library (sys_lib_dirs, usr_lib_dirs, p.prerequisite); if (pt == nullptr) @@ -437,7 +437,10 @@ namespace build2 { // Handle imported libraries. // - if (p.proj () != nullptr) + // Note that since the search is rule-specific, we don't cache the + // target in the prerequisite. + // + if (p.proj ()) pt = search_library (sys_lib_dirs, usr_lib_dirs, p.prerequisite); // The rest is the same basic logic as in search_and_match(). diff --git a/build2/cc/pkgconfig.cxx b/build2/cc/pkgconfig.cxx index 1b14960..f8d35af 100644 --- a/build2/cc/pkgconfig.cxx +++ b/build2/cc/pkgconfig.cxx @@ -41,7 +41,7 @@ namespace build2 lib& lt, liba* at, libs* st, - const string* proj, + const optional& proj, const string& stem, const dir_path& libd, const dir_paths& sysd) const @@ -56,7 +56,7 @@ namespace build2 // names). // path f; - auto search = [&f, proj, &stem, &libd] (const dir_path& dir) -> bool + auto search = [&f, &proj, &stem, &libd] (const dir_path& dir) -> bool { // Check if we have this directory inrelative to this library's // directory. @@ -93,7 +93,7 @@ namespace build2 if (exists (f)) return true; - if (proj != nullptr) + if (proj) { f = pkgd; f /= *proj; @@ -420,7 +420,7 @@ namespace build2 const string* ext (nullptr); prerequisite_key pk { - nullptr, {&lib::static_type, &out, &out, &name, ext}, &s}; + nullopt, {&lib::static_type, &out, &out, &name, ext}, &s}; if (lib* lt = static_cast (search_library (sysd, usrd, pk))) { diff --git a/build2/cc/utility.cxx b/build2/cc/utility.cxx index b786d11..f8a0afc 100644 --- a/build2/cc/utility.cxx +++ b/build2/cc/utility.cxx @@ -67,7 +67,7 @@ namespace build2 if (r == nullptr) r = &search (ls ? libs::static_type : liba::static_type, - prerequisite_key {nullptr, l.key (), nullptr}); + prerequisite_key {nullopt, l.key (), nullptr}); return *r; } diff --git a/build2/context b/build2/context index dcf1b01..cb83105 100644 --- a/build2/context +++ b/build2/context @@ -17,7 +17,6 @@ namespace build2 class file; extern string_pool extension_pool; - extern string_pool project_name_pool; // Current action (meta/operation). // diff --git a/build2/context.cxx b/build2/context.cxx index 106b7ec..a21de2a 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -24,7 +24,6 @@ using namespace butl; namespace build2 { string_pool extension_pool; - string_pool project_name_pool; const string* current_mname; const string* current_oname; @@ -58,7 +57,6 @@ namespace build2 var_pool.clear (); extension_pool.clear (); - project_name_pool.clear (); // Reset meta/operation tables. Note that the order should match the id // constants in . diff --git a/build2/dist/rule.cxx b/build2/dist/rule.cxx index e11f09f..6cbd7df 100644 --- a/build2/dist/rule.cxx +++ b/build2/dist/rule.cxx @@ -33,7 +33,7 @@ namespace build2 // Skip prerequisites imported from other projects. // - if (p.proj () != nullptr) + if (p.proj ()) continue; // If we can, go inside see-through groups. Note that here we are diff --git a/build2/file.cxx b/build2/file.cxx index cc1be77..9e957a9 100644 --- a/build2/file.cxx +++ b/build2/file.cxx @@ -826,19 +826,19 @@ namespace build2 l5 ([&]{trace << target << " from " << ibase.out_path ();}); // If there is no project specified for this target, then our run will be - // short and sweet: we simply return it as empty- project-qualified and + // short and sweet: we simply return it as empty-project-qualified and // let someone else (e.g., a rule) take a stab at it. // if (target.unqualified ()) { - target.proj = &project_name_pool.find (""); + target.proj = string (); return names {move (target)}; } // Otherwise, get the project name and convert the target to unqualified. // - const string& proj (*target.proj); - target.proj = nullptr; + string proj (move (*target.proj)); + target.proj = nullopt; scope& iroot (*ibase.root_scope ()); @@ -873,7 +873,7 @@ namespace build2 // if (out_root.empty ()) { - target.proj = &proj; + target.proj = move (proj); l5 ([&]{trace << "skipping " << target;}); return names {move (target)}; } @@ -924,7 +924,7 @@ namespace build2 // config.import.build2.b=b-boot // if (p.relative ()) - target.proj = &proj; + target.proj = move (proj); target.dir = p.directory (); target.value = p.leaf ().string (); @@ -973,7 +973,7 @@ namespace build2 // if (out_root.empty ()) { - target.proj = &proj; + target.proj = move (proj); l5 ([&]{trace << "postponing " << target;}); return names {move (target)}; } @@ -1098,7 +1098,7 @@ namespace build2 { tracer trace ("import"); - assert (pk.proj != nullptr); + assert (pk.proj); const string& p (*pk.proj); // Target type-specific search. diff --git a/build2/install/rule.cxx b/build2/install/rule.cxx index a368ac9..036f01f 100644 --- a/build2/install/rule.cxx +++ b/build2/install/rule.cxx @@ -157,7 +157,7 @@ namespace build2 // Ignore unresolved targets that are imported from other projects. // We are definitely not installing those. // - if (p.proj () != nullptr) + if (p.proj ()) continue; // Let a customized rule have its say. diff --git a/build2/name b/build2/name index 3bae2d0..2833a2b 100644 --- a/build2/name +++ b/build2/name @@ -19,9 +19,11 @@ namespace build2 // A name is what we operate on by default. Depending on the context, it can // be interpreted as a target or prerequisite name. A name without a type // and directory can be used to represent any text. A name with directory - // and empty value represents a directory. A name may also be qualified with - // a project. If the project name is empty, then it means the name is in a - // project other than our own (e.g., it is installed). + // and empty value represents a directory. + // + // A name may also be qualified with a project. If the project name is + // empty, then it means the name is in a project other than our own (e.g., + // it is installed). // // A type or project can only be specified if either directory or value are // not empty. @@ -31,7 +33,7 @@ namespace build2 // struct name { - const string* proj = nullptr; // Points to project_name_pool. + optional proj; dir_path dir; string type; string value; @@ -45,16 +47,14 @@ namespace build2 name (dir_path d, string t, string v) : dir (move (d)), type (move (t)), value (move (v)) {} - // The first argument should be from project_name_pool. - // - name (const string* p, dir_path d, string t, string v) - : proj (p), dir (move (d)), type (move (t)), value (move (v)) {} + name (optional p, dir_path d, string t, string v) + : proj (move (p)), dir (move (d)), type (move (t)), value (move (v)) {} bool - qualified () const {return proj != nullptr;} + qualified () const {return proj.has_value ();} bool - unqualified () const {return proj == nullptr;} + unqualified () const {return !qualified ();} bool typed () const {return !type.empty ();} @@ -62,7 +62,7 @@ namespace build2 bool untyped () const {return type.empty ();} - // Note: if dir and value are empty then so should be proj and type. + // Note: if dir and value are empty then there should be no proj or type. // bool empty () const {return dir.empty () && value.empty ();} diff --git a/build2/name.cxx b/build2/name.cxx index e4ecd52..0b3d00f 100644 --- a/build2/name.cxx +++ b/build2/name.cxx @@ -22,7 +22,7 @@ namespace build2 if (n.empty ()) return r; - if (n.proj != nullptr) + if (n.proj) { r += *n.proj; r += '%'; @@ -113,7 +113,7 @@ namespace build2 if (quote && n.empty ()) return os << "''"; - if (n.proj != nullptr) + if (n.proj) { write_string (*n.proj); os << '%'; diff --git a/build2/name.ixx b/build2/name.ixx index 4a8c86a..c77846a 100644 --- a/build2/name.ixx +++ b/build2/name.ixx @@ -7,13 +7,7 @@ namespace build2 inline int name:: compare (const name& x) const { - int r; - - // Project string is pooled, so for equality can just compare pointers. - // - r = proj == x.proj - ? 0 - : proj == nullptr || (x.proj != nullptr && *proj < *x.proj) ? -1 : 1; + int r (proj < x.proj ? -1 : (proj > x.proj ? 1 : 0)); if (r == 0) r = dir.compare (x.dir); diff --git a/build2/parser b/build2/parser index 5909328..f91bbb6 100644 --- a/build2/parser +++ b/build2/parser @@ -199,7 +199,7 @@ namespace build2 { names ns; parse_names ( - t, tt, ns, chunk, what, separators, 0, nullptr, nullptr, nullptr); + t, tt, ns, chunk, what, separators, 0, nullopt, nullptr, nullptr); return ns; } @@ -214,7 +214,7 @@ namespace build2 names ns; pair p ( parse_names ( - t, tt, ns, chunk, what, separators, 0, nullptr, nullptr, nullptr)); + t, tt, ns, chunk, what, separators, 0, nullopt, nullptr, nullptr)); value r (p.second); // Potentially typed NULL value. @@ -250,7 +250,7 @@ namespace build2 const char* what = "name", const string* separators = &name_separators, size_t pair = 0, - const string* prj = nullptr, + const optional& prj = nullopt, const dir_path* dir = nullptr, const string* type = nullptr); @@ -260,7 +260,7 @@ namespace build2 const char* what, const string* separators, size_t pair, - const string* prj, + const optional& prj, const dir_path* dir, const string* type); diff --git a/build2/parser.cxx b/build2/parser.cxx index 57a5618..6025082 100644 --- a/build2/parser.cxx +++ b/build2/parser.cxx @@ -2163,7 +2163,7 @@ namespace build2 const char* what, const string* separators, size_t pairn, - const string* pp, + const optional& pp, const dir_path* dp, const string* tp) { @@ -2195,7 +2195,7 @@ namespace build2 names x; // Parse into a separate list of names. parse_names_trailer ( - t, tt, x, what, separators, 0, nullptr, nullptr, nullptr); + t, tt, x, what, separators, 0, nullopt, nullptr, nullptr); if (size_t n = x.size ()) { @@ -2219,12 +2219,12 @@ namespace build2 // Move the project names. // - if (r.proj != nullptr) + if (r.proj) { - if (l.proj != nullptr) + if (l.proj) fail (loc) << "nested project name " << *r.proj; - l.proj = r.proj; + l.proj = move (r.proj); } // Merge directories. @@ -2285,7 +2285,7 @@ namespace build2 const char* what, const string* separators, size_t pairn, - const string* pp, + const optional& pp, const dir_path* dp, const string* tp) { @@ -2379,7 +2379,6 @@ namespace build2 } }; - // Number of names in the last group. This is used to detect when // we need to add an empty first pair element (e.g., @y) or when // we have a (for now unsupported) multi-name LHS (e.g., {x y}@z). @@ -2558,31 +2557,33 @@ namespace build2 // First take care of project. A project-qualified name is not very // common, so we can afford some copying for the sake of simplicity. // - const string* pp1 (pp); + optional p1; + const optional* pp1 (&pp); if (p != string::npos) { bool last (val[p] == '%'); - string::size_type p1 (last ? p : val.rfind ('%', p - 1)); + string::size_type q (last ? p : val.rfind ('%', p - 1)); - if (p1 != string::npos) + if (q != string::npos) { string proj; proj.swap (val); // First fix the rest of the name. // - val.assign (proj, p1 + 1, string::npos); - p = last ? string::npos : p - (p1 + 1); + val.assign (proj, q + 1, string::npos); + p = last ? string::npos : p - (q + 1); // Now process the project name. // - proj.resize (p1); + proj.resize (q); - if (pp != nullptr) + if (pp) fail (t) << "nested project name " << proj; - pp1 = &project_name_pool.find (proj); + p1 = move (proj); + pp1 = &p1; } } @@ -2629,7 +2630,7 @@ namespace build2 } count = parse_names_trailer ( - t, tt, ns, what, separators, pairn, pp1, dp1, tp1); + t, tt, ns, what, separators, pairn, *pp1, dp1, tp1); tt = peek (); continue; } @@ -2665,7 +2666,7 @@ namespace build2 if (dp != nullptr) dir = *dp / dir; - ns.emplace_back (pp1, + ns.emplace_back (*pp1, move (dir), (tp != nullptr ? *tp : string ()), string ()); @@ -2673,7 +2674,7 @@ namespace build2 } } - ns.emplace_back (pp1, + ns.emplace_back (*pp1, (dp != nullptr ? *dp : dir_path ()), (tp != nullptr ? *tp : string ()), move (val)); @@ -2980,14 +2981,18 @@ namespace build2 // for (const name& n: lv) { - const string* pp1 (pp); + const optional* pp1 (&pp); const dir_path* dp1 (dp); const string* tp1 (tp); - if (n.proj != 0) + optional p1; + if (n.proj) { - if (pp == nullptr) - pp1 = n.proj; + if (!pp) + { + p1 = n.proj; + pp1 = &p1; + } else fail (loc) << "nested project name " << *n.proj << " in " << what; @@ -3032,7 +3037,7 @@ namespace build2 ns.push_back (ns[pairn - 1]); } - ns.emplace_back (pp1, + ns.emplace_back (*pp1, (dp1 != nullptr ? *dp1 : dir_path ()), (tp1 != nullptr ? *tp1 : string ()), n.value); @@ -3573,7 +3578,7 @@ namespace build2 prerequisite& p ( scope_->prerequisites.insert ( - nullptr, + nullopt, dt.key (), *scope_, // Doesn't matter which scope since dir is absolute. trace).first); diff --git a/build2/prerequisite b/build2/prerequisite index 1fe3c1a..b259557 100644 --- a/build2/prerequisite +++ b/build2/prerequisite @@ -27,22 +27,21 @@ namespace build2 public: typedef build2::scope scope_type; - mutable const string* proj; // Can be NULL, from project_name_pool. - target_key tk; // .dir and .out can be relative. - mutable scope_type* scope; // Can be NULL if tk.dir is absolute. + const optional& proj; + target_key tk; // .dir and .out can be relative. + mutable scope_type* scope; // Can be NULL if tk.dir is absolute. template bool is_a () const {return tk.is_a ();} bool is_a (const target_type& tt) const {return tk.is_a (tt);} + + static const optional nullproj; }; inline bool operator== (const prerequisite_key& x, const prerequisite_key& y) { assert (x.scope == y.scope); - - // Can compare project name pointers since they are from project_name_pool. - // return x.proj == y.proj && x.tk == y.tk; } @@ -71,9 +70,7 @@ namespace std size_t operator() (const build2::prerequisite_key& k) const noexcept { - // Can hash project name pointers since they are from project_name_pool. - // - return build2::combine_hash (hash () (k.proj), + return build2::combine_hash (hash> () (k.proj), hash () (k.tk)); } }; @@ -91,7 +88,7 @@ namespace build2 // Note that unlike targets, for prerequisites an empty out directory // means undetermined rather than being definitely in the out tree. // - const string* const proj; // NULL if not project-qualified. + const optional proj; const target_type_type& type; const dir_path dir; // Normalized absolute or relative (to scope). const dir_path out; // Empty, normalized absolute, or relative. @@ -103,14 +100,14 @@ namespace build2 // a target group. public: - prerequisite (const string* p, + prerequisite (optional p, const target_type_type& t, dir_path d, dir_path o, string n, const string* e, scope_type& s) - : proj (p), + : proj (move (p)), type (t), dir (move (d)), out (move (o)), @@ -186,7 +183,7 @@ namespace build2 struct prerequisite_set: std::unordered_set { pair - insert (const string* proj, + insert (optional proj, const target_type&, dir_path dir, dir_path out, @@ -196,9 +193,10 @@ namespace build2 tracer&); pair - insert (const string* proj, const target_key& tk, scope& s, tracer& t) + insert (optional proj, const target_key& tk, scope& s, tracer& t) { - return insert (proj, *tk.type, *tk.dir, *tk.out, *tk.name, tk.ext, s, t); + return insert ( + move (proj), *tk.type, *tk.dir, *tk.out, *tk.name, tk.ext, s, t); } }; } diff --git a/build2/prerequisite.cxx b/build2/prerequisite.cxx index 5bb2ddf..82d6ea2 100644 --- a/build2/prerequisite.cxx +++ b/build2/prerequisite.cxx @@ -15,10 +15,12 @@ namespace build2 { // prerequisite_key // + const optional prerequisite_key::nullproj = nullopt; + ostream& operator<< (ostream& os, const prerequisite_key& pk) { - if (pk.proj != nullptr) + if (pk.proj) os << *pk.proj << '%'; // // Don't print scope if we are project-qualified or the prerequisite's @@ -49,7 +51,7 @@ namespace build2 // prerequisite_set // auto prerequisite_set:: - insert (const string* proj, + insert (optional proj, const target_type& tt, dir_path dir, dir_path out, @@ -64,7 +66,8 @@ namespace build2 // Find or insert. // - auto r (emplace (proj, tt, move (dir), move (out), move (name), ext, s)); + auto r ( + emplace (move (proj), tt, move (dir), move (out), move (name), ext, s)); prerequisite& p (const_cast (*r.first)); // Update extension if the existing prerequisite has it unspecified. diff --git a/build2/target b/build2/target index dd9461c..48c5ebc 100644 --- a/build2/target +++ b/build2/target @@ -748,12 +748,14 @@ namespace build2 return target != nullptr ? target->name : prerequisite.get ().name; } - const string* + const optional& proj () const { // Target cannot be project-qualified. // - return target != nullptr ? nullptr : prerequisite.get ().proj; + return target != nullptr + ? prerequisite_key::nullproj + : prerequisite.get ().proj; } target_type& diff --git a/build2/target.ixx b/build2/target.ixx index e9d2d7a..2d4fff8 100644 --- a/build2/target.ixx +++ b/build2/target.ixx @@ -29,7 +29,7 @@ namespace build2 // The use of the group's prerequisite scope is debatable. // scope& s (prerequisite.get ().scope); - return s.prerequisites.insert (nullptr, key ().tk, s, trace).first; + return s.prerequisites.insert (nullopt, key ().tk, s, trace).first; } // prerequisite_members -- cgit v1.1