From 8efa0d147bca1d08cc5a4752895ecce2c53186ba Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 16 Dec 2015 14:14:23 +0000 Subject: [PATCH] Review feedback --- probe/process/walker.go | 11 ++++------- probe/process/walker_test.go | 17 +++++++++-------- render/detailed_node.go | 2 +- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/probe/process/walker.go b/probe/process/walker.go index 620268462..36eaa9c89 100644 --- a/probe/process/walker.go +++ b/probe/process/walker.go @@ -20,7 +20,7 @@ type Walker interface { // CachingWalker is a walker than caches a copy of the output from another // Walker, and then allows other concurrent readers to Walk that copy. type CachingWalker struct { - cache []Process + cache map[int]Process previousByPID map[int]Process cacheLock sync.RWMutex source Walker @@ -47,9 +47,9 @@ func (c *CachingWalker) Walk(f func(Process, Process)) error { // Tick updates cached copy of process list func (c *CachingWalker) Tick() error { - newCache := []Process{} + newCache := map[int]Process{} err := c.source.Walk(func(p, _ Process) { - newCache = append(newCache, p) + newCache[p.PID] = p }) if err != nil { return err @@ -57,10 +57,7 @@ func (c *CachingWalker) Tick() error { c.cacheLock.Lock() defer c.cacheLock.Unlock() - c.previousByPID = map[int]Process{} - for _, p := range c.cache { - c.previousByPID[p.PID] = p - } + c.previousByPID = c.cache c.cache = newCache return nil } diff --git a/probe/process/walker_test.go b/probe/process/walker_test.go index f12e0aa26..f5e448c0e 100644 --- a/probe/process/walker_test.go +++ b/probe/process/walker_test.go @@ -34,15 +34,16 @@ func TestCache(t *testing.T) { t.Fatal(err) } + want, err := all(walker) have, err := all(cachingWalker) - if err != nil || !reflect.DeepEqual(processes, have) { - t.Errorf("%v (%v)", test.Diff(processes, have), err) + if err != nil || !reflect.DeepEqual(want, have) { + t.Errorf("%v (%v)", test.Diff(want, have), err) } walker.processes = []process.Process{} have, err = all(cachingWalker) - if err != nil || !reflect.DeepEqual(processes, have) { - t.Errorf("%v (%v)", test.Diff(processes, have), err) + if err != nil || !reflect.DeepEqual(want, have) { + t.Errorf("%v (%v)", test.Diff(want, have), err) } err = cachingWalker.Tick() @@ -51,16 +52,16 @@ func TestCache(t *testing.T) { } have, err = all(cachingWalker) - want := []process.Process{} + want = map[process.Process]struct{}{} if err != nil || !reflect.DeepEqual(want, have) { t.Errorf("%v (%v)", test.Diff(want, have), err) } } -func all(w process.Walker) ([]process.Process, error) { - all := []process.Process{} +func all(w process.Walker) (map[process.Process]struct{}, error) { + all := map[process.Process]struct{}{} err := w.Walk(func(p, _ process.Process) { - all = append(all, p) + all[p] = struct{}{} }) return all, err } diff --git a/render/detailed_node.go b/render/detailed_node.go index 26ca8c086..bd516e86e 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -539,7 +539,7 @@ func hostOriginTable(nmd report.Node) (Table, bool) { fmt formatter }{ {host.CPUUsage, "CPU Usage", formatPercent}, - {host.MemUsage, "Memory Usage", formatPercent}, + {host.MemUsage, "Memory Usage", formatMemory}, } { if val, ok := nmd.Metrics[tuple.key]; ok { rows = append(rows, sparklineRow(tuple.human, val, tuple.fmt))