From 10c6e2a6295660df4bc5d11077d8b4c0f2bb0871 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sat, 8 Mar 2008 19:21:20 +0000
Subject: [PATCH] * Combine referrer update to prevent quadratic complexity in
 the   garbage collector.

---
 src/libstore/local-store.cc | 62 +++++++++++++++++++++++++------------
 src/libstore/local-store.hh |  7 +++++
 src/libutil/util.hh         |  4 +++
 3 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index b9d76c573..eeb830199 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -82,6 +82,11 @@ LocalStore::LocalStore()
 
 LocalStore::~LocalStore()
 {
+    try {
+        flushDelayedUpdates();
+    } catch (...) {
+        ignoreException();
+    }
 }
 
 
@@ -211,21 +216,28 @@ static void appendReferrer(const Path & from, const Path & to)
 }
 
 
-/* Atomically update the referrers file. */
-static void rewriteReferrers(const Path & path, const PathSet & referrers)
+/* Atomically update the referrers file.  If `purge' is true, the set
+   of referrers is set to `referrers'.  Otherwise, the current set of
+   referrers is purged of invalid paths. */
+void LocalStore::rewriteReferrers(const Path & path, bool purge, PathSet referrers)
 {
     Path referrersFile = referrersFileFor(path);
     
     PathLocks referrersLock(singleton<PathSet, Path>(referrersFile));
     referrersLock.setDeletion(true);
 
+    if (purge)
+        /* queryReferrers() purges invalid paths, so that's all we
+           need. */
+        queryReferrers(path, referrers);
+
     Path tmpFile = tmpFileForAtomicUpdate(referrersFile);
     
     AutoCloseFD fd = open(tmpFile.c_str(), O_WRONLY | O_TRUNC | O_CREAT, 0666);
     if (fd == -1) throw SysError(format("opening file `%1%'") % referrersFile);
     
     string s;
-    for (PathSet::const_iterator i = referrers.begin(); i != referrers.end(); ++i) {
+    foreach (PathSet::const_iterator, i, referrers) {
         s += " "; s += *i;
     }
     
@@ -238,6 +250,15 @@ static void rewriteReferrers(const Path & path, const PathSet & referrers)
 }
 
 
+void LocalStore::flushDelayedUpdates()
+{
+    foreach (PathSet::iterator, i, delayedUpdates) {
+        rewriteReferrers(*i, true, PathSet());
+    }
+    delayedUpdates.clear();
+}
+
+
 void LocalStore::registerValidPath(const Path & path,
     const Hash & hash, const PathSet & references,
     const Path & deriver)
@@ -501,10 +522,6 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
 }
 
 
-#define foreach(it_type, it, collection)                                \
-    for (it_type it = collection.begin(); it != collection.end(); ++it)
-
-
 /* Invalidate a path.  The caller is responsible for checking that
    there are no referrers. */
 void LocalStore::invalidatePath(const Path & path)
@@ -529,18 +546,25 @@ void LocalStore::invalidatePath(const Path & path)
 
     /* Clear `path' from the info cache. */
     pathInfoCache.erase(path);
+    delayedUpdates.erase(path);
 
-    /* Remove the corresponding referrer entries for each path
-       referenced by this one.  This has to happen after removing the
-       info file to preserve the invariant (see
-       registerValidPath()). */
-    foreach (PathSet::iterator, i, info.references) {
-        /* !!! O(n) */
-        if (*i == path) continue; /* self-reference */
-        PathSet referrers; queryReferrers(*i, referrers);
-        referrers.erase(path);
-        rewriteReferrers(*i, referrers);
-    }
+    /* Cause the referrer files for each path referenced by this one
+       to be updated.  This has to happen after removing the info file
+       to preserve the invariant (see registerValidPath()).
+
+       The referrer files are updated lazily in flushDelayedUpdates()
+       to prevent quadratic performance in the garbage collector
+       (i.e., when N referrers to some path X are deleted, we have to
+       rewrite the referrers file for X N times, causing O(N^2) I/O).
+
+       What happens if we die before the referrer file can be updated?
+       That's not a problem, because stale (invalid) entries in the
+       referrer file are ignored by queryReferrers().  Thus a referrer
+       file is allowed to have stale entries; removing them is just an
+       optimisation.  verifyStore() gets rid of them eventually.
+    */
+    foreach (PathSet::iterator, i, info.references)
+        if (*i != path) delayedUpdates.insert(*i);
 }
 
 
@@ -943,7 +967,7 @@ void LocalStore::verifyStore(bool checkContents)
             } else referrersNew.insert(*j);
         }
 
-        if (update) rewriteReferrers(from, referrersNew);
+        if (update) rewriteReferrers(from, false, referrersNew);
     }
 }
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 3c7102e90..65bec1047 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -118,10 +118,17 @@ private:
        every once in a while. */
     std::map<Path, ValidPathInfo> pathInfoCache;
 
+    /* Store paths for which the referrers file must be purged. */
+    PathSet delayedUpdates;
+
     void registerValidPath(const ValidPathInfo & info);
 
     ValidPathInfo queryPathInfo(const Path & path);
 
+    void rewriteReferrers(const Path & path, bool purge, PathSet referrers);
+
+    void flushDelayedUpdates();
+    
     bool queryReferrersInternal(const Path & path, PathSet & referrers);
     
     void invalidatePath(const Path & path);
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 657f45ced..2609c2fee 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -12,6 +12,10 @@
 namespace nix {
 
 
+#define foreach(it_type, it, collection)                                \
+    for (it_type it = collection.begin(); it != collection.end(); ++it)
+
+
 /* Return an environment variable. */
 string getEnv(const string & key, const string & def = "");