From 99b73fb5071db9fd757c9927fc3fde34e2abac63 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 9 Oct 2019 15:51:52 +0200 Subject: [PATCH] OCD performance fix: {find,count}+insert => insert --- src/libexpr/eval.cc | 24 +++++++------------ src/libexpr/get-drvs.cc | 3 +-- src/libexpr/parser.y | 3 +-- src/libexpr/primops.cc | 6 ++--- src/libexpr/value-to-xml.cc | 5 ++-- src/libstore/build.cc | 11 ++++----- src/libstore/builtins/buildenv.cc | 3 +-- src/libstore/local-store.cc | 3 +-- src/libstore/misc.cc | 9 +++---- src/libstore/references.cc | 3 +-- src/libstore/store-api.cc | 3 +-- src/nix-env/nix-env.cc | 7 ++---- src/nix-store/dotgraph.cc | 7 ++---- src/nix-store/nix-store.cc | 3 +-- src/nix/verify.cc | 3 +-- .../resolve-system-dependencies.cc | 4 +--- 16 files changed, 32 insertions(+), 65 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9f4b6b411..f60e8d3ab 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -46,11 +46,10 @@ static void printValue(std::ostream & str, std::set & active, con { checkInterrupt(); - if (active.find(&v) != active.end()) { + if (!active.insert(&v).second) { str << ""; return; } - active.insert(&v); switch (v.type) { case tInt: @@ -1446,8 +1445,7 @@ void EvalState::forceValueDeep(Value & v) std::function recurse; recurse = [&](Value & v) { - if (seen.find(&v) != seen.end()) return; - seen.insert(&v); + if (!seen.insert(&v).second) return; forceValue(v); @@ -1865,8 +1863,7 @@ size_t valueSize(Value & v) std::set seen; auto doString = [&](const char * s) -> size_t { - if (seen.find(s) != seen.end()) return 0; - seen.insert(s); + if (!seen.insert(s).second) return 0; return strlen(s) + 1; }; @@ -1874,8 +1871,7 @@ size_t valueSize(Value & v) std::function doEnv; doValue = [&](Value & v) -> size_t { - if (seen.find(&v) != seen.end()) return 0; - seen.insert(&v); + if (!seen.insert(&v).second) return 0; size_t sz = sizeof(Value); @@ -1890,8 +1886,7 @@ size_t valueSize(Value & v) sz += doString(v.path); break; case tAttrs: - if (seen.find(v.attrs) == seen.end()) { - seen.insert(v.attrs); + if (seen.insert(v.attrs).second) { sz += sizeof(Bindings) + sizeof(Attr) * v.attrs->capacity(); for (auto & i : *v.attrs) sz += doValue(*i.value); @@ -1900,8 +1895,7 @@ size_t valueSize(Value & v) case tList1: case tList2: case tListN: - if (seen.find(v.listElems()) == seen.end()) { - seen.insert(v.listElems()); + if (seen.insert(v.listElems()).second) { sz += v.listSize() * sizeof(Value *); for (size_t n = 0; n < v.listSize(); ++n) sz += doValue(*v.listElems()[n]); @@ -1922,8 +1916,7 @@ size_t valueSize(Value & v) sz += doValue(*v.primOpApp.right); break; case tExternal: - if (seen.find(v.external) != seen.end()) break; - seen.insert(v.external); + if (!seen.insert(v.external).second) break; sz += v.external->valueSize(seen); break; default: @@ -1934,8 +1927,7 @@ size_t valueSize(Value & v) }; doEnv = [&](Env & env) -> size_t { - if (seen.find(&env) != seen.end()) return 0; - seen.insert(&env); + if (!seen.insert(&env).second) return 0; size_t sz = sizeof(Env) + sizeof(Value *) * env.size; diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 21a4d7917..4e22a1d77 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -277,8 +277,7 @@ static bool getDerivation(EvalState & state, Value & v, /* Remove spurious duplicates (e.g., a set like `rec { x = derivation {...}; y = x;}'. */ - if (done.find(v.attrs) != done.end()) return false; - done.insert(v.attrs); + if (!done.insert(v.attrs).second) return false; DrvInfo drv(state, attrPath, v.attrs); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 967c88d9b..1dcd532ad 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -138,11 +138,10 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, static void addFormal(const Pos & pos, Formals * formals, const Formal & formal) { - if (formals->argNames.find(formal.name) != formals->argNames.end()) + if (!formals->argNames.insert(formal.name).second) throw ParseError(format("duplicate formal function argument '%1%' at %2%") % formal.name % pos); formals->formals.push_front(formal); - formals->argNames.insert(formal.name); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d4c60f870..deaffa605 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -396,8 +396,7 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar throw EvalError(format("attribute 'key' required, at %1%") % pos); state.forceValue(*key->value); - if (doneKeys.find(key->value) != doneKeys.end()) continue; - doneKeys.insert(key->value); + if (!doneKeys.insert(key->value).second) continue; res.push_back(e); /* Call the `operator' function with `e' as argument. */ @@ -1273,13 +1272,12 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args, string name = state.forceStringNoCtx(*j->value, pos); Symbol sym = state.symbols.create(name); - if (seen.find(sym) == seen.end()) { + if (seen.insert(sym).second) { Bindings::iterator j2 = v2.attrs->find(state.symbols.create(state.sValue)); if (j2 == v2.attrs->end()) throw TypeError(format("'value' attribute missing in a call to 'listToAttrs', at %1%") % pos); v.attrs->push_back(Attr(sym, j2->value, j2->pos)); - seen.insert(sym); } } diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 00b1918a8..1f0b1541d 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -105,10 +105,9 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, XMLOpenElement _(doc, "derivation", xmlAttrs); - if (drvPath != "" && drvsSeen.find(drvPath) == drvsSeen.end()) { - drvsSeen.insert(drvPath); + if (drvPath != "" && drvsSeen.insert(drvPath).second) showAttrs(state, strict, location, *v.attrs, doc, context, drvsSeen); - } else + else doc.writeEmptyElement("repeated"); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9a6729f9e..0e0f8a545 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -568,10 +568,9 @@ UserLock::UserLock() { auto lockedPaths(lockedPaths_.lock()); - if (lockedPaths->count(fnUserLock)) + if (!lockedPaths->insert(fnUserLock).second) /* We already have a lock on this one. */ continue; - lockedPaths->insert(fnUserLock); } try { @@ -620,8 +619,8 @@ UserLock::UserLock() UserLock::~UserLock() { auto lockedPaths(lockedPaths_.lock()); - assert(lockedPaths->count(fnUserLock)); - lockedPaths->erase(fnUserLock); + auto erased = lockedPaths->erase(fnUserLock); + assert(erased); } @@ -1125,10 +1124,8 @@ void DerivationGoal::addWantedOutputs(const StringSet & outputs) needRestart = true; } else for (auto & i : outputs) - if (wantedOutputs.find(i) == wantedOutputs.end()) { - wantedOutputs.insert(i); + if (wantedOutputs.insert(i).second) needRestart = true; - } } diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 74e706664..096593886 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -123,8 +123,7 @@ static Path out; static void addPkg(const Path & pkgDir, int priority) { - if (done.count(pkgDir)) return; - done.insert(pkgDir); + if (!done.insert(pkgDir).second) return; createLinks(pkgDir, out, priority); try { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index a2af51d0e..859094e61 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1292,8 +1292,7 @@ void LocalStore::verifyPath(const Path & path, const PathSet & store, { checkInterrupt(); - if (done.find(path) != done.end()) return; - done.insert(path); + if (!done.insert(path).second) return; if (!isStorePath(path)) { printError(format("path '%1%' is not in the Nix store") % path); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index dddf13430..05b93d4c9 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -29,8 +29,7 @@ void Store::computeFSClosure(const PathSet & startPaths, { auto state(state_.lock()); if (state->exc) return; - if (state->paths.count(path)) return; - state->paths.insert(path); + if (!state->paths.insert(path).second) return; state->pending++; } @@ -175,8 +174,7 @@ void Store::queryMissing(const PathSet & targets, { auto state(state_.lock()); - if (state->done.count(path)) return; - state->done.insert(path); + if (!state->done.insert(path).second) return; } DrvPathWithOutputs i2 = parseDrvPathWithOutputs(path); @@ -252,8 +250,7 @@ Paths Store::topoSortPaths(const PathSet & paths) if (parents.find(path) != parents.end()) throw BuildError(format("cycle detected in the references of '%1%' from '%2%'") % path % *parent); - if (visited.find(path) != visited.end()) return; - visited.insert(path); + if (!visited.insert(path).second) return; parents.insert(path); PathSet references; diff --git a/src/libstore/references.cc b/src/libstore/references.cc index 5b7eb1f84..0dcc264c3 100644 --- a/src/libstore/references.cc +++ b/src/libstore/references.cc @@ -36,11 +36,10 @@ static void search(const unsigned char * s, size_t len, } if (!match) continue; string ref((const char *) s + i, refLength); - if (hashes.find(ref) != hashes.end()) { + if (hashes.erase(ref)) { debug(format("found reference to '%1%' at offset '%2%'") % ref % i); seen.insert(ref); - hashes.erase(ref); } ++i; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 5f63c53b5..be13fa49a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -954,8 +954,7 @@ std::list> getDefaultSubstituters() StringSet done; auto addStore = [&](const std::string & uri) { - if (done.count(uri)) return; - done.insert(uri); + if (!done.insert(uri).second) return; try { stores.push_back(openStore(uri)); } catch (Error & e) { diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 87b2e43f0..48686ce72 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -124,11 +124,10 @@ static void getAllExprs(EvalState & state, string attrName = i; if (hasSuffix(attrName, ".nix")) attrName = string(attrName, 0, attrName.size() - 4); - if (attrs.find(attrName) != attrs.end()) { + if (!attrs.insert(attrName).second) { printError(format("warning: name collision in input Nix expressions, skipping '%1%'") % path2); continue; } - attrs.insert(attrName); /* Load the expression on demand. */ Value & vFun = state.getBuiltin("import"); Value & vArg(*state.allocValue()); @@ -307,10 +306,8 @@ static DrvInfos filterBySelector(EvalState & state, const DrvInfos & allElems, /* Insert only those elements in the final list that we haven't inserted before. */ for (auto & j : matches) - if (done.find(j.second) == done.end()) { - done.insert(j.second); + if (done.insert(j.second).second) elems.push_back(j.first); - } } checkSelectorUse(selectors); diff --git a/src/nix-store/dotgraph.cc b/src/nix-store/dotgraph.cc index abdfa5e58..d448654fe 100644 --- a/src/nix-store/dotgraph.cc +++ b/src/nix-store/dotgraph.cc @@ -71,9 +71,7 @@ void printClosure(const Path & nePath, const StoreExpr & fs) Path path = *(workList.begin()); workList.erase(path); - if (doneSet.find(path) == doneSet.end()) { - doneSet.insert(path); - + if (doneSet.insert(path).second) { ClosureElems::const_iterator elem = fs.closure.elems.find(path); if (elem == fs.closure.elems.end()) throw Error(format("bad closure, missing path '%1%'") % path); @@ -104,8 +102,7 @@ void printDotGraph(ref store, const PathSet & roots) Path path = *(workList.begin()); workList.erase(path); - if (doneSet.find(path) != doneSet.end()) continue; - doneSet.insert(path); + if (!doneSet.insert(path).second) continue; cout << makeNode(path, symbolicName(path), "#ff0000"); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 0cbceb02f..19384499d 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -242,11 +242,10 @@ const string treeNull = " "; static void printTree(const Path & path, const string & firstPad, const string & tailPad, PathSet & done) { - if (done.find(path) != done.end()) { + if (!done.insert(path).second) { cout << format("%1%%2% [...]\n") % firstPad % path; return; } - done.insert(path); cout << format("%1%%2%\n") % firstPad % path; diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 7ef571561..2e0d5cffe 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -118,8 +118,7 @@ struct CmdVerify : StorePathsCommand auto doSigs = [&](StringSet sigs) { for (auto sig : sigs) { - if (sigsSeen.count(sig)) continue; - sigsSeen.insert(sig); + if (!sigsSeen.insert(sig).second) continue; if (validSigs < ValidPathInfo::maxSigs && info->checkSignature(publicKeys, sig)) validSigs++; } diff --git a/src/resolve-system-dependencies/resolve-system-dependencies.cc b/src/resolve-system-dependencies/resolve-system-dependencies.cc index e02cfc03e..45e8b6796 100644 --- a/src/resolve-system-dependencies/resolve-system-dependencies.cc +++ b/src/resolve-system-dependencies/resolve-system-dependencies.cc @@ -117,9 +117,7 @@ Path resolveSymlink(const Path & path) std::set resolveTree(const Path & path, PathSet & deps) { std::set results; - if (deps.count(path)) - return {}; - deps.insert(path); + if (!deps.insert(path).second) return {}; for (auto & lib : runResolver(path)) { results.insert(lib); for (auto & p : resolveTree(lib, deps)) {