Remove pointer math (comparison) from render caching, as it is unreli…

…able

app/benchmark_internal_test.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/ugorji/go/codec"
 
-	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/report"
 	"github.com/weaveworks/scope/test/fixture"
 )
@@ -51,7 +50,11 @@ func BenchmarkTopologyList(b *testing.B) {
 		Form: url.Values{},
 	}
 	for i := 0; i < b.N; i++ {
-		render.ResetCache()
+		b.StopTimer()
+		topologyRegistry.walk(func(t APITopologyDesc) {
+			t.renderer.ResetCache()
+		})
+		b.StartTimer()
 		topologyRegistry.renderTopologies(report, request)
 	}
 }

render/benchmark_test.go

@@ -62,7 +62,9 @@ func benchmarkRender(b *testing.B, r render.Renderer) {
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
-		render.ResetCache()
+		b.StopTimer()
+		r.ResetCache()
+		b.StartTimer()
 		benchmarkRenderResult = r.Render(report)
 		if len(benchmarkRenderResult) == 0 {
 			b.Errorf("Rendered topology contained no nodes")
@@ -80,7 +82,9 @@ func benchmarkStats(b *testing.B, r render.Renderer) {
 
 	for i := 0; i < b.N; i++ {
 		// No way to tell if this was successful :(
-		render.ResetCache()
+		b.StopTimer()
+		r.ResetCache()
+		b.StartTimer()
 		benchmarkStatsResult = r.Stats(report)
 	}
 }

render/memoise.go

@@ -1,44 +1,33 @@
 package render
 
 import (
-	"fmt"
-	"reflect"
-
 	"github.com/bluele/gcache"
 
 	"github.com/weaveworks/scope/report"
 )
 
-var renderCache = gcache.New(100).LRU().Build()
-
 type memoise struct {
 	Renderer
+	cache gcache.Cache
 }
 
 // Memoise wraps the renderer in a loving embrace of caching
-func Memoise(r Renderer) Renderer { return &memoise{r} }
+func Memoise(r Renderer) Renderer { return &memoise{r, gcache.New(10).LRU().Build()} }
 
 // Render produces a set of RenderableNodes given a Report.
 // Ideally, it just retrieves it from the cache, otherwise it calls through to
 // `r` and stores the result.
 func (m *memoise) Render(rpt report.Report) RenderableNodes {
-	key := ""
-	v := reflect.ValueOf(m.Renderer)
-	switch v.Kind() {
-	case reflect.Ptr, reflect.Func:
-		key = fmt.Sprintf("%s-%x", rpt.ID, v.Pointer())
-	default:
-		return m.Renderer.Render(rpt)
-	}
-	if result, err := renderCache.Get(key); err == nil {
+	if result, err := m.cache.Get(rpt.ID); err == nil {
 		return result.(RenderableNodes)
 	}
 	output := m.Renderer.Render(rpt)
-	renderCache.Set(key, output)
+	m.cache.Set(rpt.ID, output)
 	return output
 }
 
 // ResetCache blows away the rendered node cache.
-func ResetCache() {
-	renderCache.Purge()
+func (m *memoise) ResetCache() {
+	m.cache.Purge()
+	m.Renderer.ResetCache()
 }

render/memoise_test.go

@@ -13,6 +13,7 @@ type renderFunc func(r report.Report) render.RenderableNodes
 
 func (f renderFunc) Render(r report.Report) render.RenderableNodes { return f(r) }
 func (f renderFunc) Stats(r report.Report) render.Stats            { return render.Stats{} }
+func (f renderFunc) ResetCache()                                   {}
 
 func TestMemoise(t *testing.T) {
 	calls := 0
@@ -48,4 +49,13 @@ func TestMemoise(t *testing.T) {
 	if calls != 2 {
 		t.Errorf("Expected renderer to have been called again for a different report")
 	}
+
+	m.ResetCache()
+	result4 := m.Render(rpt1)
+	if !reflect.DeepEqual(result1, result4) {
+		t.Errorf("Expected original result to be returned: %s", test.Diff(result1, result4))
+	}
+	if calls != 3 {
+		t.Errorf("Expected renderer to have been called again after cache reset")
+	}
 }

render/render.go

@@ -8,6 +8,7 @@ import (
 type Renderer interface {
 	Render(report.Report) RenderableNodes
 	Stats(report.Report) Stats
+	ResetCache()
 }
 
 // Stats is the type returned by Renderer.Stats
@@ -49,6 +50,13 @@ func (r *Reduce) Stats(rpt report.Report) Stats {
 	return result
 }
 
+// ResetCache blows away the rendered node cache.
+func (r *Reduce) ResetCache() {
+	for _, renderer := range *r {
+		renderer.ResetCache()
+	}
+}
+
 // Map is a Renderer which produces a set of RenderableNodes from the set of
 // RenderableNodes produced by another Renderer.
 type Map struct {

render/render_test.go

@@ -13,12 +13,9 @@ type mockRenderer struct {
 	render.RenderableNodes
 }
 
-func (m mockRenderer) Render(rpt report.Report) render.RenderableNodes {
-	return m.RenderableNodes
-}
-func (m mockRenderer) Stats(rpt report.Report) render.Stats {
-	return render.Stats{}
-}
+func (m mockRenderer) Render(rpt report.Report) render.RenderableNodes { return m.RenderableNodes }
+func (m mockRenderer) Stats(rpt report.Report) render.Stats            { return render.Stats{} }
+func (m mockRenderer) ResetCache()                                     {}
 
 func TestReduceRender(t *testing.T) {
 	renderer := render.Reduce([]render.Renderer{

render/selectors.go

@@ -18,6 +18,9 @@ func (t TopologySelector) Stats(r report.Report) Stats {
 	return Stats{}
 }
 
+// ResetCache implements Renderer
+func (t TopologySelector) ResetCache() {}
+
 // MakeRenderableNodes converts a topology to a set of RenderableNodes
 func MakeRenderableNodes(t report.Topology) RenderableNodes {
 	result := RenderableNodes{}