From 94d52f02a7012301573305e8b80a5872a5b8284a Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 27 Jan 2016 12:04:48 +0000 Subject: [PATCH] Refactored render/memoise and added a basic test for it --- render/filters.go | 4 ++-- render/memoise.go | 44 ++++++++++++++++++++++++++++++++++++ render/memoise_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++ render/render.go | 37 ++++-------------------------- 4 files changed, 101 insertions(+), 35 deletions(-) create mode 100644 render/memoise.go create mode 100644 render/memoise_test.go diff --git a/render/filters.go b/render/filters.go index 9feda297d..1f9f785ca 100644 --- a/render/filters.go +++ b/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 diff --git a/render/memoise.go b/render/memoise.go new file mode 100644 index 000000000..6bee0c647 --- /dev/null +++ b/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() +} diff --git a/render/memoise_test.go b/render/memoise_test.go new file mode 100644 index 000000000..fe9948555 --- /dev/null +++ b/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") + } +} diff --git a/render/render.go b/render/render.go index f2e3ec6da..a57f99be5 100644 --- a/render/render.go +++ b/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