Only calculate slave->build mapping once in json output.

third_party/buildbot_8_4p1/buildbot/status/web/status_json.py

Index: third_party/buildbot_8_4p1/buildbot/status/web/status_json.py
diff --git a/third_party/buildbot_8_4p1/buildbot/status/web/status_json.py b/third_party/buildbot_8_4p1/buildbot/status/web/status_json.py
index b656563257e3b0dbe89ca0e3fe26b330c1e7d99b..0573578a5b80a50f19954edfe9b1d00ac04600f9 100644
--- a/third_party/buildbot_8_4p1/buildbot/status/web/status_json.py
+++ b/third_party/buildbot_8_4p1/buildbot/status/web/status_json.py
@@ -414,12 +414,14 @@ class BuilderSlavesJsonResources(JsonResource):
     pageTitle = 'BuilderSlaves'
 
     def __init__(self, status, builder_status):
+        buildcache = {}

[Isaac (away), 2013/04/04 21:24:38]

Maybe:

self.buildcache = collections.defaultdict(dict) 

         JsonResource.__init__(self, status)
         self.builder_status = builder_status
         for slave_name in self.builder_status.slavenames:
             self.putChild(slave_name,
                           SlaveJsonResource(status,
-                                            self.status.getSlave(slave_name)))
+                                            self.status.getSlave(slave_name),
+                                            buildcache=buildcache))

[Isaac (away), 2013/04/04 21:24:38]

buildcache=self.buildcache

[Mike Stip (use stip instead), 2013/04/05 23:35:04]

It's probably not a good idea to persist this cache past init -- it should
really only be used during construction and then discarded.

 
 
 class BuildJsonResource(JsonResource):
@@ -620,12 +622,17 @@ class SlaveJsonResource(JsonResource):
 """
     pageTitle = 'Slave'
 
-    def __init__(self, status, slave_status):
+    def __init__(self, status, slave_status, buildcache=None):

[Isaac (away), 2013/04/04 21:24:38]

why optional?

[Mike Stip (use stip instead), 2013/04/05 23:35:04]

It's only needed if you are repeatedly creating SlaveJsonResources.

[Isaac (away), 2013/04/05 23:45:01]

As far as I can tell, the only clients are in this class and they use the param.
I believe option params are discouraged unless there is a present need for them.

         JsonResource.__init__(self, status)
         self.slave_status = slave_status
         self.name = self.slave_status.getName()
         self.builders = None
 
+        # buildcache is used to cache build information across multiple
+        # invoations of SlaveJsonResource. It should be set to an empty {}.
+        # Without it, each invocation will do a full build scan.
+        self.buildcache = buildcache
+
     def getBuilders(self):
         if self.builders is None:
             # Figure out all the builders to which it's attached
@@ -635,10 +642,10 @@ class SlaveJsonResource(JsonResource):
                     self.builders.append(builderName)
         return self.builders
 
-    def asDict(self, request):
-        results = self.slave_status.asDict()
-        # Enhance it by adding more informations.
-        results['builders'] = {}
+    def mapSlavesToBuilds(self):

[Isaac (away), 2013/04/04 21:24:38]

I think this would be cleaner if it buildcache[self.name] (and lazily creates
the rest of buildcache)

[Mike Stip (use stip instead), 2013/04/05 23:35:04]

The scan works by creating all slaves at once, so creating "the rest of
buildcache" doesn't make sense. It's all or nothing. I've modified this to do
the lazy build outside of mapSlavesToBuilds.

+        if self.buildcache:
+            return
+        self.buildcache = {}

[Isaac (away), 2013/04/04 21:24:38]

646-648 can be removed if param not optional.

         for builderName in self.getBuilders():
             builds = []
             builder_status = self.status.getBuilder(builderName)
@@ -647,9 +654,16 @@ class SlaveJsonResource(JsonResource):
                 if not build_status or not build_status.isFinished():
                     # If not finished, it will appear in runningBuilds.
                     break
-                if build_status.getSlavename() == self.name:
-                    builds.append(build_status.getNumber())
-            results['builders'][builderName] = builds
+                slave = self.buildcache.get(build_status.getSlavename(), {})

[Isaac (away), 2013/04/04 21:24:38]

if buildcache is defaultdict. 657-660 become:

slave = self.buildcache[build_status.getSlavename()]
slave.setdefault(builderName, []).append(build_status.getNumber())

+                self.buildcache[build_status.getSlavename()] = slave
+                slave[builderName] = slave.get(builderName, [])
+                slave[builderName].append(build_status.getNumber())
+

[Isaac (away), 2013/04/04 21:24:38]

return self.buildcache[self.name]

+    def asDict(self, request):
+        results = self.slave_status.asDict()
+        # Enhance it by adding more informations.
+        self.mapSlavesToBuilds()
+        results['builders'] = self.buildcache.get(self.name, {})

[Isaac (away), 2013/04/04 21:24:38]

results['builders'] = getSlavesToBuildMap()

         return results
 
 
@@ -659,11 +673,13 @@ class SlavesJsonResource(JsonResource):
     pageTitle = 'Slaves'
 
     def __init__(self, status):
+        buildcache = {}

[Isaac (away), 2013/04/04 21:24:38]

Maybe self.buildcache = collections.defaultdict(dict)

[Mike Stip (use stip instead), 2013/04/05 23:35:04]

Done.

         JsonResource.__init__(self, status)
         for slave_name in status.getSlaveNames():
             self.putChild(slave_name,
                           SlaveJsonResource(status,
-                                            status.getSlave(slave_name)))
+                                            status.getSlave(slave_name),
+                                            buildcache=buildcache))

[Isaac (away), 2013/04/04 21:24:38]

self.buildcache

 
 
 class SourceStampJsonResource(JsonResource):