Switch to the good LRU implementation.

third_party/buildbot_8_4p1/buildbot/status/builder.py

Index: third_party/buildbot_8_4p1/buildbot/status/builder.py
===================================================================
--- third_party/buildbot_8_4p1/buildbot/status/builder.py	(revision 127129)
+++ third_party/buildbot_8_4p1/buildbot/status/builder.py	(working copy)
@@ -86,8 +86,10 @@
         self.currentBuilds = []
         self.nextBuild = None
         self.watchers = []
-        self.buildCache = weakref.WeakValueDictionary()
-        self.buildCache_LRU = []
+        #self.buildCache = weakref.WeakValueDictionary()

[M-A Ruel, 2012/03/16 13:02:06]

I'd simply remove the old lines instead of commenting them.

[cmp, 2012/03/16 17:40:51]

Agree.

[szager, 2012/03/16 20:52:04]

Done.

+        #self.buildCache_LRU = []
+        self.buildCache = util.lru.AsyncLRUCache(self.cacheMiss,
+                                                 self.buildCacheSize)
         self.logCompressionLimit = False # default to no compression for tests
         self.logCompressionMethod = "bz2"
         self.logMaxSize = None # No default limit
@@ -103,7 +105,7 @@
         d = styles.Versioned.__getstate__(self)
         d['watchers'] = []
         del d['buildCache']
-        del d['buildCache_LRU']
+        #del d['buildCache_LRU']
         for b in self.currentBuilds:
             b.saveYourself()
             # TODO: push a 'hey, build was interrupted' event
@@ -119,8 +121,10 @@
         # when loading, re-initialize the transient stuff. Remember that
         # upgradeToVersion1 and such will be called after this finishes.
         styles.Versioned.__setstate__(self, d)
-        self.buildCache = weakref.WeakValueDictionary()
-        self.buildCache_LRU = []
+        #self.buildCache = weakref.WeakValueDictionary()
+        #self.buildCache_LRU = []
+        self.buildCache = util.lru.AsyncLRUCache(self.cacheMiss,
+                                                 self.buildCacheSize)
         self.currentBuilds = []
         self.watchers = []
         self.slavenames = []
@@ -132,6 +136,7 @@
         # gets pickled and unpickled.
         if buildmaster.buildCacheSize is not None:
             self.buildCacheSize = buildmaster.buildCacheSize
+            self.buildCache.set_max_size(buildmaster.buildCacheSize)
 
     def upgradeToVersion1(self):
         if hasattr(self, 'slavename'):
@@ -186,33 +191,24 @@
         except:
             log.msg("unable to save builder %s" % self.name)
             log.err()
-        
 
+
     # build cache management
 
     def makeBuildFilename(self, number):
         return os.path.join(self.basedir, "%d" % number)
 
-    def touchBuildCache(self, build):
-        self.buildCache[build.number] = build
-        if build in self.buildCache_LRU:
-            self.buildCache_LRU.remove(build)
-        self.buildCache_LRU = self.buildCache_LRU[-(self.buildCacheSize-1):] + [ build ]
-        return build
+#    def touchBuildCache(self, build):
+#        self.buildCache[build.number] = build
+#        if build in self.buildCache_LRU:
+#            self.buildCache_LRU.remove(build)
+#        self.buildCache_LRU = self.buildCache_LRU[-(self.buildCacheSize-1):] + [ build ]
+#        return build
 
     def getBuildByNumber(self, number):
-        # first look in currentBuilds
-        for b in self.currentBuilds:
-            if b.number == number:
-                return self.touchBuildCache(b)
+        return self.buildCache.get(number)
 
-        # then in the buildCache
-        if number in self.buildCache:
-            metrics.MetricCountEvent.log("buildCache.hits", 1)
-            return self.touchBuildCache(self.buildCache[number])
-        metrics.MetricCountEvent.log("buildCache.misses", 1)
-
-        # then fall back to loading it from disk
+    def loadBuildFromFile(self, number):
         filename = self.makeBuildFilename(number)
         try:
             log.msg("Loading builder %s's build %d from on-disk pickle"
@@ -235,12 +231,23 @@
             build.upgradeLogfiles()
             # check that logfiles exist
             build.checkLogfiles()
-            return self.touchBuildCache(build)
+            #return self.touchBuildCache(build)
+            return build
         except IOError:
             raise IndexError("no such build %d" % number)
         except EOFError:
             raise IndexError("corrupted build pickle %d" % number)
 
+    def cacheMiss(self, number):
+        # first look in currentBuilds
+        for b in self.currentBuilds:

[M-A Ruel, 2012/03/16 13:02:06]

if any(b.number == number for b in self.currentBuilds):

[szager, 2012/03/16 20:52:04]

I don't like that because it requires two linear searches through
self.currentBuilds -- once for 'any', and then again to return the value.

+            if b.number == number:
+                #return self.touchBuildCache(b)
+                return defer.succeed(b)
+
+        # then fall back to loading it from disk
+        return threads.deferToThread(self.loadBuildFromFile, number)
+
     def prune(self, events_only=False):
         # begin by pruning our own events
         self.events = self.events[-self.eventHorizon:]
@@ -287,7 +294,7 @@
                     is_logfile = True
 
             if num is None: continue
-            if num in self.buildCache: continue
+            if num in self.buildCache.cache: continue
 
             if (is_logfile and num < earliest_log) or num < earliest_build:
                 pathname = os.path.join(self.basedir, filename)
@@ -510,7 +517,8 @@
         assert s.number == self.nextBuildNumber - 1
         assert s not in self.currentBuilds
         self.currentBuilds.append(s)
-        self.touchBuildCache(s)
+        #self.touchBuildCache(s)
+        self.buildCache.put(s.number, s)
 
         # now that the BuildStatus is prepared to answer queries, we can
         # announce the new build to all our watchers
@@ -620,7 +628,7 @@
         # Collect build numbers.
         # Important: Only grab the *cached* builds numbers to reduce I/O.
         current_builds = [b.getNumber() for b in self.currentBuilds]
-        cached_builds = list(set(self.buildCache.keys() + current_builds))
+        cached_builds = list(set(self.buildCache.cache.keys() + current_builds))
         cached_builds.sort()
         result['cachedBuilds'] = cached_builds
         result['currentBuilds'] = current_builds