Merge pull request #861 from weaveworks/refactor-memoise

Refactored render/memoise and added a basic test for it

render/filters.go

@@ -62,7 +62,7 @@ type Filter struct {
 
 // MakeFilter makes a new Filter.
 func MakeFilter(f func(RenderableNode) bool, r Renderer) Renderer {
-	return &Filter{r, f}
+	return Memoise(&Filter{r, f})
 }
 
 // Render implements Renderer
@@ -75,7 +75,7 @@ func (f *Filter) render(rpt report.Report) (RenderableNodes, int) {
 	output := RenderableNodes{}
 	inDegrees := map[string]int{}
 	filtered := 0
-	for id, node := range memoisedRender(f.Renderer, rpt) {
+	for id, node := range f.Renderer.Render(rpt) {
 		if f.FilterFunc(node) {
 			output[id] = node
 			inDegrees[id] = 0

render/memoise.go

@@ -0,0 +1,44 @@
+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
+}
+
+// Memoise wraps the renderer in a loving embrace of caching
+func Memoise(r Renderer) Renderer { return &memoise{r} }
+
+// 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 {
+		return result.(RenderableNodes)
+	}
+	output := m.Renderer.Render(rpt)
+	renderCache.Set(key, output)
+	return output
+}
+
+// ResetCache blows away the rendered node cache.
+func ResetCache() {
+	renderCache.Purge()
+}

render/memoise_test.go

@@ -0,0 +1,51 @@
+package render_test
+
+import (
+	"testing"
+
+	"github.com/weaveworks/scope/render"
+	"github.com/weaveworks/scope/report"
+	"github.com/weaveworks/scope/test"
+	"github.com/weaveworks/scope/test/reflect"
+)
+
+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 TestMemoise(t *testing.T) {
+	calls := 0
+	r := renderFunc(func(rpt report.Report) render.RenderableNodes {
+		calls++
+		return render.RenderableNodes{rpt.ID: render.NewRenderableNode(rpt.ID)}
+	})
+	m := render.Memoise(r)
+	rpt1 := report.MakeReport()
+
+	result1 := m.Render(rpt1)
+	// it should have rendered it.
+	if _, ok := result1[rpt1.ID]; !ok {
+		t.Errorf("Expected rendered report to contain a node, but got: %v", result1)
+	}
+	if calls != 1 {
+		t.Errorf("Expected renderer to have been called the first time")
+	}
+
+	result2 := m.Render(rpt1)
+	if !reflect.DeepEqual(result1, result2) {
+		t.Errorf("Expected memoised result to be returned: %s", test.Diff(result1, result2))
+	}
+	if calls != 1 {
+		t.Errorf("Expected renderer to not have been called the second time")
+	}
+
+	rpt2 := report.MakeReport()
+	result3 := m.Render(rpt2)
+	if reflect.DeepEqual(result1, result3) {
+		t.Errorf("Expected different result for different report, but were the same")
+	}
+	if calls != 2 {
+		t.Errorf("Expected renderer to have been called again for a different report")
+	}
+}

render/render.go

@@ -1,38 +1,9 @@
 package render
 
 import (
-	"fmt"
-	"reflect"
-
-	"github.com/bluele/gcache"
-
 	"github.com/weaveworks/scope/report"
 )
 
-var renderCache = gcache.New(100).LRU().Build()
-
-func memoisedRender(r Renderer, rpt report.Report) RenderableNodes {
-	key := ""
-	v := reflect.ValueOf(r)
-	switch v.Kind() {
-	case reflect.Ptr, reflect.Func:
-		key = fmt.Sprintf("%s-%x", rpt.ID, v.Pointer())
-	default:
-		return r.Render(rpt)
-	}
-	if result, err := renderCache.Get(key); err == nil {
-		return result.(RenderableNodes)
-	}
-	output := r.Render(rpt)
-	renderCache.Set(key, output)
-	return output
-}
-
-// ResetCache blows away the rendered node cache.
-func ResetCache() {
-	renderCache.Purge()
-}
-
 // Renderer is something that can render a report to a set of RenderableNodes.
 type Renderer interface {
 	Render(report.Report) RenderableNodes
@@ -57,14 +28,14 @@ type Reduce []Renderer
 // MakeReduce is the only sane way to produce a Reduce Renderer.
 func MakeReduce(renderers ...Renderer) Renderer {
 	r := Reduce(renderers)
-	return &r
+	return Memoise(&r)
 }
 
 // Render produces a set of RenderableNodes given a Report.
 func (r *Reduce) Render(rpt report.Report) RenderableNodes {
 	result := RenderableNodes{}
 	for _, renderer := range *r {
-		result = result.Merge(memoisedRender(renderer, rpt))
+		result = result.Merge(renderer.Render(rpt))
 	}
 	return result
 }
@@ -87,7 +58,7 @@ type Map struct {
 
 // MakeMap makes a new Map
 func MakeMap(f MapFunc, r Renderer) Renderer {
-	return &Map{f, r}
+	return Memoise(&Map{f, r})
 }
 
 // Render transforms a set of RenderableNodes produces by another Renderer.
@@ -107,7 +78,7 @@ func (m *Map) Stats(rpt report.Report) Stats {
 
 func (m *Map) render(rpt report.Report) (RenderableNodes, map[string]report.IDList) {
 	var (
-		input         = memoisedRender(m.Renderer, rpt)
+		input         = m.Renderer.Render(rpt)
 		output        = RenderableNodes{}
 		mapped        = map[string]report.IDList{} // input node ID -> output node IDs
 		adjacencies   = map[string]report.IDList{} // output node ID -> input node Adjacencies