From 4eee079528a7c3da8c2fa4ef3349a2560109a184 Mon Sep 17 00:00:00 2001 From: Leonid Gershanovich Date: Mon, 6 Jan 2020 20:34:04 +0000 Subject: [PATCH 1/6] refactor Group.Do implement via Group.Dochan call --- singleflight/singleflight.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/singleflight/singleflight.go b/singleflight/singleflight.go index 97a1aa4..be4bdbe 100644 --- a/singleflight/singleflight.go +++ b/singleflight/singleflight.go @@ -49,23 +49,8 @@ type Result struct { // original to complete and receives the same results. // The return value shared indicates whether v was given to multiple callers. func (g *Group) Do(key string, fn func() (interface{}, error)) (v interface{}, err error, shared bool) { - g.mu.Lock() - if g.m == nil { - g.m = make(map[string]*call) - } - if c, ok := g.m[key]; ok { - c.dups++ - g.mu.Unlock() - c.wg.Wait() - return c.val, c.err, true - } - c := new(call) - c.wg.Add(1) - g.m[key] = c - g.mu.Unlock() - - g.doCall(c, key, fn) - return c.val, c.err, c.dups > 0 + r := <-g.DoChan(key, fn) + return r.Val, r.Err, r.Shared } // DoChan is like Do but returns a channel that will receive the From d2a98c94d4d999fcb6dfef0e023c7b6de07c7ffd Mon Sep 17 00:00:00 2001 From: Leonid Gershanovich Date: Mon, 6 Jan 2020 20:37:32 +0000 Subject: [PATCH 2/6] rename call.dups to call.refCount --- singleflight/singleflight.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/singleflight/singleflight.go b/singleflight/singleflight.go index be4bdbe..efa804f 100644 --- a/singleflight/singleflight.go +++ b/singleflight/singleflight.go @@ -24,8 +24,8 @@ type call struct { // These fields are read and written with the singleflight // mutex held before the WaitGroup is done, and are read but // not written after the WaitGroup is done. - dups int - chans []chan<- Result + refCount int64 + chans []chan<- Result } // Group represents a class of work and forms a namespace in @@ -62,12 +62,12 @@ func (g *Group) DoChan(key string, fn func() (interface{}, error)) <-chan Result g.m = make(map[string]*call) } if c, ok := g.m[key]; ok { - c.dups++ + c.refCount++ c.chans = append(c.chans, ch) g.mu.Unlock() return ch } - c := &call{chans: []chan<- Result{ch}} + c := &call{refCount: 1, chans: []chan<- Result{ch}} c.wg.Add(1) g.m[key] = c g.mu.Unlock() @@ -87,7 +87,7 @@ func (g *Group) doCall(c *call, key string, fn func() (interface{}, error)) { delete(g.m, key) } for _, ch := range c.chans { - ch <- Result{c.val, c.err, c.dups > 0} + ch <- Result{c.val, c.err, c.refCount > 1} } g.mu.Unlock() } From f08e0ceafe2aa4d4a7bc299a4c001751c4d67844 Mon Sep 17 00:00:00 2001 From: Leonid Gershanovich Date: Mon, 6 Jan 2020 20:57:09 +0000 Subject: [PATCH 3/6] replace "shared bool", as part of Do/DoChan return, with refShared, which carries both boolean "shared" indicator as well as actual reference counter --- singleflight/singleflight.go | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/singleflight/singleflight.go b/singleflight/singleflight.go index efa804f..ae295d7 100644 --- a/singleflight/singleflight.go +++ b/singleflight/singleflight.go @@ -7,6 +7,7 @@ package singleflight // import "golang.org/x/sync/singleflight" import "sync" +import "sync/atomic" // call is an in-flight or completed singleflight.Do call type call struct { @@ -40,15 +41,31 @@ type Group struct { type Result struct { Val interface{} Err error - Shared bool + Shared refShared +} + +// this encapsulates both "shared boolean" as well as actual reference counter +// callers can call refShared.Decrement to determine when last caller is done using result, so cleanup if needed can be performed +type refShared struct { + shared bool + refCount *int64 +} + +// Decrement will atomically decrement refcounter and will return new value +func (rs *refShared) Decrement() int64 { + return atomic.AddInt64(rs.refCount, -1) +} + +func (rs *refShared) Shared() bool { + return rs.shared } // Do executes and returns the results of the given function, making // sure that only one execution is in-flight for a given key at a // time. If a duplicate comes in, the duplicate caller waits for the // original to complete and receives the same results. -// The return value shared indicates whether v was given to multiple callers. -func (g *Group) Do(key string, fn func() (interface{}, error)) (v interface{}, err error, shared bool) { +// The return value shared indicates whether v was given to multiple callers (and a reference counter for callers too). +func (g *Group) Do(key string, fn func() (interface{}, error)) (v interface{}, err error, shared refShared) { r := <-g.DoChan(key, fn) return r.Val, r.Err, r.Shared } @@ -86,8 +103,10 @@ func (g *Group) doCall(c *call, key string, fn func() (interface{}, error)) { if !c.forgotten { delete(g.m, key) } + //shared := newRefShared(&c.refCount) + shared := refShared{shared: c.refCount > 1, refCount: &c.refCount} for _, ch := range c.chans { - ch <- Result{c.val, c.err, c.refCount > 1} + ch <- Result{c.val, c.err, shared} } g.mu.Unlock() } From 8b166cb5b63e1682865de27eec366d2d5def7dc6 Mon Sep 17 00:00:00 2001 From: Leonid Gershanovich Date: Mon, 6 Jan 2020 20:58:01 +0000 Subject: [PATCH 4/6] singleflight_test: add rudimentary checking for refShared --- singleflight/singleflight_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/singleflight/singleflight_test.go b/singleflight/singleflight_test.go index ad04037..a4d6ea3 100644 --- a/singleflight/singleflight_test.go +++ b/singleflight/singleflight_test.go @@ -15,7 +15,7 @@ import ( func TestDo(t *testing.T) { var g Group - v, err, _ := g.Do("key", func() (interface{}, error) { + v, err, shared := g.Do("key", func() (interface{}, error) { return "bar", nil }) if got, want := fmt.Sprintf("%v (%T)", v, v), "bar (string)"; got != want { @@ -24,6 +24,12 @@ func TestDo(t *testing.T) { if err != nil { t.Errorf("Do error = %v", err) } + if shared.Decrement() != 0 { + t.Errorf("ref counter is expected to be 0") + } + if shared.Shared() { + t.Errorf("Do returned shared") + } } func TestDoErr(t *testing.T) { From 682143fb42c027d6fb30e38bec411140a7b27124 Mon Sep 17 00:00:00 2001 From: Leonid Gershanovich Date: Mon, 6 Jan 2020 21:54:33 +0000 Subject: [PATCH 5/6] refShared -> exported RefShared --- singleflight/singleflight.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/singleflight/singleflight.go b/singleflight/singleflight.go index ae295d7..b1877c9 100644 --- a/singleflight/singleflight.go +++ b/singleflight/singleflight.go @@ -41,22 +41,24 @@ type Group struct { type Result struct { Val interface{} Err error - Shared refShared + Shared RefShared } // this encapsulates both "shared boolean" as well as actual reference counter -// callers can call refShared.Decrement to determine when last caller is done using result, so cleanup if needed can be performed -type refShared struct { +// callers can call RefShared.Decrement to determine when last caller is done using result, so cleanup if needed can be performed +type RefShared struct { shared bool refCount *int64 } // Decrement will atomically decrement refcounter and will return new value -func (rs *refShared) Decrement() int64 { +func (rs *RefShared) Decrement() int64 { return atomic.AddInt64(rs.refCount, -1) } -func (rs *refShared) Shared() bool { +// returns boolean indicator of whether original "ref counter" had more than 1 reference +// it will return same value regardless of whether Decrement() method was called +func (rs *RefShared) Shared() bool { return rs.shared } @@ -65,7 +67,7 @@ func (rs *refShared) Shared() bool { // time. If a duplicate comes in, the duplicate caller waits for the // original to complete and receives the same results. // The return value shared indicates whether v was given to multiple callers (and a reference counter for callers too). -func (g *Group) Do(key string, fn func() (interface{}, error)) (v interface{}, err error, shared refShared) { +func (g *Group) Do(key string, fn func() (interface{}, error)) (v interface{}, err error, shared RefShared) { r := <-g.DoChan(key, fn) return r.Val, r.Err, r.Shared } @@ -104,7 +106,7 @@ func (g *Group) doCall(c *call, key string, fn func() (interface{}, error)) { delete(g.m, key) } //shared := newRefShared(&c.refCount) - shared := refShared{shared: c.refCount > 1, refCount: &c.refCount} + shared := RefShared{shared: c.refCount > 1, refCount: &c.refCount} for _, ch := range c.chans { ch <- Result{c.val, c.err, shared} } From 55a436903c480ec4d5e635eefec4759f3861dd3c Mon Sep 17 00:00:00 2001 From: Leonid Gershanovich Date: Mon, 6 Jan 2020 21:55:25 +0000 Subject: [PATCH 6/6] feedback on import --- singleflight/singleflight.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/singleflight/singleflight.go b/singleflight/singleflight.go index b1877c9..cdaad2d 100644 --- a/singleflight/singleflight.go +++ b/singleflight/singleflight.go @@ -6,8 +6,10 @@ // mechanism. package singleflight // import "golang.org/x/sync/singleflight" -import "sync" -import "sync/atomic" +import ( + "sync" + "sync/atomic" +) // call is an in-flight or completed singleflight.Do call type call struct { @@ -44,7 +46,7 @@ type Result struct { Shared RefShared } -// this encapsulates both "shared boolean" as well as actual reference counter +// RefShared struct encapsulates both "shared boolean" as well as actual reference counter // callers can call RefShared.Decrement to determine when last caller is done using result, so cleanup if needed can be performed type RefShared struct { shared bool @@ -105,7 +107,6 @@ func (g *Group) doCall(c *call, key string, fn func() (interface{}, error)) { if !c.forgotten { delete(g.m, key) } - //shared := newRefShared(&c.refCount) shared := RefShared{shared: c.refCount > 1, refCount: &c.refCount} for _, ch := range c.chans { ch <- Result{c.val, c.err, shared}