From 117670be570d775a18e4e35db3dae00abc24f729 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 Mar 2012 20:43:33 +0200 Subject: [PATCH] Make the garbage collector more concurrent Make the garbage collector more concurrent by deleting valid paths outside the region where we're holding the global GC lock. This should greatly reduce the time during which new builds are blocked, since the deletion accounts for the vast majority of the time spent in the GC. To ensure that this is safe, the valid paths are invalidated and renamed to some arbitrary path while we're holding the lock. This ensures that we when we finally delete the path, it's not a (newly) valid or locked path. --- src/libstore/gc.cc | 51 +++++++++++++++++++++++++++++-------- src/libstore/local-store.cc | 7 +---- src/libstore/local-store.hh | 9 ++++--- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index d81bf40c7..65c8dc210 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -1,6 +1,7 @@ #include "globals.hh" #include "misc.hh" #include "local-store.hh" +#include "immutable.hh" #include @@ -396,11 +397,11 @@ struct LocalStore::GCState PathSet deleted; PathSet live; PathSet busy; + PathSet invalidated; bool gcKeepOutputs; bool gcKeepDerivations; - GCState(GCResults & results_) : results(results_) - { - } + unsigned long long bytesInvalidated; + GCState(GCResults & results_) : results(results_), bytesInvalidated(0) { } }; @@ -419,6 +420,16 @@ bool LocalStore::isActiveTempFile(const GCState & state, } +void LocalStore::deleteGarbage(GCState & state, const Path & path) +{ + printMsg(lvlInfo, format("deleting `%1%'") % path); + unsigned long long bytesFreed, blocksFreed; + deletePathWrapped(path, bytesFreed, blocksFreed); + state.results.bytesFreed += bytesFreed; + state.results.blocksFreed += blocksFreed; +} + + bool LocalStore::tryToDelete(GCState & state, const Path & path) { checkInterrupt(); @@ -502,15 +513,27 @@ bool LocalStore::tryToDelete(GCState & state, const Path & path) /* The path is garbage, so delete it. */ if (shouldDelete(state.options.action)) { - printMsg(lvlInfo, format("deleting `%1%'") % path); - unsigned long long bytesFreed, blocksFreed; - deleteFromStore(path, bytesFreed, blocksFreed); - state.results.bytesFreed += bytesFreed; - state.results.blocksFreed += blocksFreed; + if (isValidPath(path)) { + /* If it's a valid path, invalidate it, rename it, and + schedule it for deletion. The renaming is to ensure + that later (when we're not holding the global GC lock) + we can delete the path without being afraid that the + path has become alive again. */ + printMsg(lvlInfo, format("invalidating `%1%'") % path); + // Estimate the amount freed using the narSize field. + state.bytesInvalidated += queryPathInfo(path).narSize; + invalidatePathChecked(path); + Path tmp = (format("%1%-gc-%2%") % path % getpid()).str(); + makeMutable(path.c_str()); + if (rename(path.c_str(), tmp.c_str())) + throw SysError(format("unable to rename `%1%' to `%2%'") % path % tmp); + state.invalidated.insert(tmp); + } else + deleteGarbage(state, path); - if (state.options.maxFreed && state.results.bytesFreed > state.options.maxFreed) { - printMsg(lvlInfo, format("deleted more than %1% bytes; stopping") % state.options.maxFreed); + if (state.options.maxFreed && state.results.bytesFreed + state.bytesInvalidated > state.options.maxFreed) { + printMsg(lvlInfo, format("deleted or invalidated more than %1% bytes; stopping") % state.options.maxFreed); throw GCLimitReached(); } @@ -635,6 +658,14 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) } catch (GCLimitReached & e) { } } + + /* Allow other processes to add to the store from here on. */ + fdGCLock.close(); + + /* Delete the invalidated paths now that the lock has been + released. */ + foreach (PathSet::iterator, i, state.invalidated) + deleteGarbage(state, *i); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index f3b779dd0..1ab9d15eb 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1380,11 +1380,8 @@ Paths LocalStore::importPaths(bool requireSignature, Source & source) } -void LocalStore::deleteFromStore(const Path & path, unsigned long long & bytesFreed, - unsigned long long & blocksFreed) +void LocalStore::invalidatePathChecked(const Path & path) { - bytesFreed = 0; - assertStorePath(path); while (1) { @@ -1404,8 +1401,6 @@ void LocalStore::deleteFromStore(const Path & path, unsigned long long & bytesFr break; } catch (SQLiteBusy & e) { }; } - - deletePathWrapped(path, bytesFreed, blocksFreed); } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 2739c4eea..8e3cbe5ce 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -164,10 +164,6 @@ public: void collectGarbage(const GCOptions & options, GCResults & results); - /* Delete a path from the Nix store. */ - void deleteFromStore(const Path & path, unsigned long long & bytesFreed, - unsigned long long & blocksFreed); - /* Optimise the disk space usage of the Nix store by hard-linking files with the same contents. */ void optimiseStore(bool dryRun, OptimiseStats & stats); @@ -238,6 +234,9 @@ private: void invalidatePath(const Path & path); + /* Delete a path from the Nix store. */ + void invalidatePathChecked(const Path & path); + void verifyPath(const Path & path, const PathSet & store, PathSet & done, PathSet & validPaths); @@ -249,6 +248,8 @@ private: struct GCState; + void deleteGarbage(GCState & state, const Path & path); + bool tryToDelete(GCState & state, const Path & path); bool isActiveTempFile(const GCState & state,