From 3cb17425390f8b476015172ca55d422eadae9c07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Wed, 27 Nov 2019 13:56:43 -0500 Subject: [PATCH 01/11] Simplify keys and resultmap structs --- dataloader_test.go | 29 +++++++++++-------- key.go | 69 +++++++++++++++++++++++----------------------- result.go | 48 +++++++++++++++----------------- result_test.go | 4 +-- trace.go | 2 +- 5 files changed, 78 insertions(+), 74 deletions(-) diff --git a/dataloader_test.go b/dataloader_test.go index 6ced464..7a16a97 100644 --- a/dataloader_test.go +++ b/dataloader_test.go @@ -32,45 +32,52 @@ func getBatchFunction(cb func(), result dataloader.Result) dataloader.BatchFunct return func(ctx context.Context, keys dataloader.Keys) *dataloader.ResultMap { cb() m := dataloader.NewResultMap(1) - m.Set(keys.Keys()[0].(PrimaryKey).String(), result) + m.Set(keys.Keys()[0].(PrimaryKey), result) return &m } } // ========================= mock cache ========================= +type cacheEntry struct { + Key dataloader.Key + Value dataloader.Result +} + type mockCache struct { - r map[string]dataloader.Result + r map[string]cacheEntry } func newMockCache(cap int) dataloader.Cache { - return &mockCache{r: make(map[string]dataloader.Result, cap)} + return &mockCache{r: make(map[string]cacheEntry, cap)} } func (c *mockCache) SetResult(ctx context.Context, key dataloader.Key, result dataloader.Result) { - c.r[key.String()] = result + c.r[key.String()] = cacheEntry{key, result} } func (c *mockCache) SetResultMap(ctx context.Context, resultMap dataloader.ResultMap) { - for _, k := range resultMap.Keys() { - c.r[k] = resultMap.GetValueForString(k) + for k, v := range resultMap { + c.r[k.String()] = cacheEntry{k, v} } } func (c *mockCache) GetResult(ctx context.Context, key dataloader.Key) (dataloader.Result, bool) { r, ok := c.r[key.String()] - return r, ok + return r.Value, ok } func (c *mockCache) GetResultMap(ctx context.Context, keys ...dataloader.Key) (dataloader.ResultMap, bool) { + var nok bool result := dataloader.NewResultMap(len(keys)) for _, k := range keys { r, ok := c.r[k.String()] if !ok { - return dataloader.NewResultMap(len(keys)), false + nok = true + continue } - result.Set(k.String(), r) + result.Set(r.Key, r.Value) } - return result, true + return result, !nok } func (c *mockCache) Delete(ctx context.Context, key dataloader.Key) bool { @@ -83,7 +90,7 @@ func (c *mockCache) Delete(ctx context.Context, key dataloader.Key) bool { } func (c *mockCache) ClearAll(ctx context.Context) bool { - c.r = make(map[string]dataloader.Result, len(c.r)) + c.r = make(map[string]cacheEntry, len(c.r)) return true } diff --git a/key.go b/key.go index d59e0ed..2c75d46 100644 --- a/key.go +++ b/key.go @@ -13,71 +13,72 @@ type Key interface { } // Keys wraps an array of keys and contains accessor methods -type Keys interface { - Append(...Key) - Capacity() int - Length() int - ClearAll() - // Keys returns a an array of unique results after calling Raw on each key - Keys() []interface{} - IsEmpty() bool +type Keys []Key + +type StringKey string + +func (k StringKey) String() string { + return string(k) } -type keys struct { - k []Key +func (k StringKey) Raw() interface{} { + return k } // NewKeys returns a new instance of the Keys array with the provided capacity. func NewKeys(capacity int) Keys { - return &keys{ - k: make([]Key, 0, capacity), - } + return make([]Key, 0, capacity) } // NewKeysWith is a helper method for returning a new keys array which includes the // the provided keys func NewKeysWith(key ...Key) Keys { - return &keys{ - k: key, - } + return append([]Key{}, key...) } // ================================== public methods ================================== -func (k *keys) Append(keys ...Key) { +func (k *Keys) Append(keys ...Key) { for _, key := range keys { + for _, kk := range *k { // skip duplicates + if kk == key { + return + } + } if key != nil && key.Raw() != nil { // don't track nil keys - k.k = append(k.k, key) + *k = append(*k, key) } } } -func (k *keys) Capacity() int { - return cap(k.k) +func (k Keys) Capacity() int { + return cap(k) } -func (k *keys) Length() int { - return len(k.k) +func (k Keys) Length() int { + return len(k) } -func (k *keys) ClearAll() { - k.k = make([]Key, 0, len(k.k)) +func (k *Keys) ClearAll() { + *k = []Key{} } -func (k *keys) Keys() []interface{} { +func (k *Keys) Keys() []interface{} { result := make([]interface{}, 0, k.Length()) - temp := make(map[Key]bool, k.Length()) - - for _, val := range k.k { - if _, ok := temp[val]; !ok { - temp[val] = true - result = append(result, val.Raw()) - } + for _, val := range *k { + result = append(result, val.Raw()) } + return result +} +func (k *Keys) StringKeys() []string { + result := make([]string, 0, k.Length()) + for _, val := range *k { + result = append(result, val.String()) + } return result } -func (k *keys) IsEmpty() bool { - return len(k.k) == 0 +func (k *Keys) IsEmpty() bool { + return len(*k) == 0 } diff --git a/result.go b/result.go index 252bd85..6b0e18e 100644 --- a/result.go +++ b/result.go @@ -7,58 +7,54 @@ type Result struct { } // ResultMap maps each loaded elements Result against the elements unique identifier (Key) -type ResultMap interface { - Set(string, Result) - GetValue(Key) (Result, bool) - Length() int - // Keys returns a slice of all unique identifiers used in the containing map (keys) - Keys() []string - GetValueForString(string) Result -} - -type resultMap struct { - r map[string]Result -} +type ResultMap map[Key]Result // NewResultMap returns a new instance of the result map with the provided capacity. // Each value defaults to nil func NewResultMap(capacity int) ResultMap { - r := make(map[string]Result, capacity) - - return &resultMap{r: r} + r := make(map[Key]Result, capacity) + return r } // ===================================== public methods ===================================== // Set adds the value to the to the result set. -func (r *resultMap) Set(identifier string, value Result) { - r.r[identifier] = value +func (r ResultMap) Set(identifier Key, value Result) { + r[identifier] = value } // GetValue returns the value from the results for the provided key and true // if the value was found, otherwise false. -func (r *resultMap) GetValue(key Key) (Result, bool) { +func (r ResultMap) GetValue(key Key) (Result, bool) { if key == nil { return Result{}, false } - result, ok := r.r[key.String()] + result, ok := r[key] return result, ok } -func (r *resultMap) GetValueForString(key string) Result { +func (r ResultMap) GetValueForString(key StringKey) Result { // No need to check ok, missing value from map[Any]interface{} is nil by default. - return r.r[key] + return r[key] } -func (r *resultMap) Keys() []string { - res := make([]string, 0, len(r.r)) - for k := range r.r { +func (r ResultMap) Keys() []Key { + res := make([]Key, 0, len(r)) + for k, _ := range r { res = append(res, k) } return res } -func (r *resultMap) Length() int { - return len(r.r) +func (r ResultMap) StringKeys() []string { + res := make([]string, 0, len(r)) + for k, _ := range r { + res = append(res, k.String()) + } + return res +} + +func (r ResultMap) Length() int { + return len(r) } diff --git a/result_test.go b/result_test.go index ccd92c1..490b5af 100644 --- a/result_test.go +++ b/result_test.go @@ -14,7 +14,7 @@ func TestEnsureOKForResult(t *testing.T) { rmap := dataloader.NewResultMap(2) key := PrimaryKey(1) value := dataloader.Result{Result: 1, Err: nil} - rmap.Set(key.String(), value) + rmap.Set(key, value) // invoke/assert result, ok := rmap.GetValue(key) @@ -27,7 +27,7 @@ func TestEnsureNotOKForResult(t *testing.T) { key := PrimaryKey(1) key2 := PrimaryKey(2) value := dataloader.Result{Result: 1, Err: nil} - rmap.Set(key.String(), value) + rmap.Set(key, value) // invoke/assert result, ok := rmap.GetValue(key2) diff --git a/trace.go b/trace.go index b61e431..958c7bf 100644 --- a/trace.go +++ b/trace.go @@ -82,7 +82,7 @@ func (*openTracer) Batch(ctx context.Context) (context.Context, BatchFinishFunc) span, spanCtx := opentracing.StartSpanFromContext(ctx, "Dataloader: batch") return spanCtx, func(r ResultMap) { - span.SetTag("keys", fmt.Sprintf("[%s]", strings.Join(r.Keys(), ", "))) + span.SetTag("keys", fmt.Sprintf("[%s]", strings.Join(r.StringKeys(), ", "))) span.Finish() } } From 8692ebb0f1b5e01671b04f70bd439177a059bc23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Wed, 27 Nov 2019 14:29:01 -0500 Subject: [PATCH 02/11] Speed up append keys --- key.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/key.go b/key.go index 2c75d46..a04e531 100644 --- a/key.go +++ b/key.go @@ -40,14 +40,15 @@ func NewKeysWith(key ...Key) Keys { func (k *Keys) Append(keys ...Key) { for _, key := range keys { + if key == nil && key.Raw() == nil { // don't track nil keys + continue + } for _, kk := range *k { // skip duplicates if kk == key { return } } - if key != nil && key.Raw() != nil { // don't track nil keys - *k = append(*k, key) - } + *k = append(*k, key) } } From e421e5542f3f7c0e2ffd523a749c9f30b8470535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Wed, 27 Nov 2019 14:37:17 -0500 Subject: [PATCH 03/11] Maintain same size when clearing keys array --- key.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/key.go b/key.go index a04e531..ec78895 100644 --- a/key.go +++ b/key.go @@ -61,7 +61,7 @@ func (k Keys) Length() int { } func (k *Keys) ClearAll() { - *k = []Key{} + *k = make([]Key, 0, len(*k)) } func (k *Keys) Keys() []interface{} { From 352ed53965ccc8bb673544572ecf68dad494ed56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Wed, 27 Nov 2019 18:37:51 -0500 Subject: [PATCH 04/11] Revert Keys to mask underlying array + clean up ResultMap --- dataloader_test.go | 29 +++++++++---------- key.go | 42 +++++++++++++++++----------- result.go | 22 +++++---------- strategies/once/once_test.go | 4 +-- strategies/sozu/sozu.go | 2 +- strategies/sozu/sozu_test.go | 20 ++++++------- strategies/standard/standard.go | 5 +++- strategies/standard/standard_test.go | 12 ++++---- trace.go | 2 +- 9 files changed, 69 insertions(+), 69 deletions(-) diff --git a/dataloader_test.go b/dataloader_test.go index 7a16a97..9f491fb 100644 --- a/dataloader_test.go +++ b/dataloader_test.go @@ -38,59 +38,56 @@ func getBatchFunction(cb func(), result dataloader.Result) dataloader.BatchFunct } // ========================= mock cache ========================= -type cacheEntry struct { - Key dataloader.Key - Value dataloader.Result -} - type mockCache struct { - r map[string]cacheEntry + r map[string]dataloader.Result } func newMockCache(cap int) dataloader.Cache { - return &mockCache{r: make(map[string]cacheEntry, cap)} + return &mockCache{r: make(map[string]dataloader.Result, cap)} } func (c *mockCache) SetResult(ctx context.Context, key dataloader.Key, result dataloader.Result) { - c.r[key.String()] = cacheEntry{key, result} + c.r[key.String()] = result } func (c *mockCache) SetResultMap(ctx context.Context, resultMap dataloader.ResultMap) { for k, v := range resultMap { - c.r[k.String()] = cacheEntry{k, v} + c.r[k] = v } } func (c *mockCache) GetResult(ctx context.Context, key dataloader.Key) (dataloader.Result, bool) { r, ok := c.r[key.String()] - return r.Value, ok + return r, ok } func (c *mockCache) GetResultMap(ctx context.Context, keys ...dataloader.Key) (dataloader.ResultMap, bool) { var nok bool result := dataloader.NewResultMap(len(keys)) - for _, k := range keys { - r, ok := c.r[k.String()] + for _, key := range keys { + var k = key.String() + r, ok := c.r[k] if !ok { nok = true continue } - result.Set(r.Key, r.Value) + result[k] = r } return result, !nok } func (c *mockCache) Delete(ctx context.Context, key dataloader.Key) bool { - _, ok := c.r[key.String()] + var k = key.String() + _, ok := c.r[k] if ok { - delete(c.r, key.String()) + delete(c.r, k) return true } return false } func (c *mockCache) ClearAll(ctx context.Context) bool { - c.r = make(map[string]cacheEntry, len(c.r)) + c.r = make(map[string]dataloader.Result, len(c.r)) return true } diff --git a/key.go b/key.go index ec78895..dd0e86a 100644 --- a/key.go +++ b/key.go @@ -13,7 +13,9 @@ type Key interface { } // Keys wraps an array of keys and contains accessor methods -type Keys []Key +type Keys struct { + keys []Key +} type StringKey string @@ -27,59 +29,65 @@ func (k StringKey) Raw() interface{} { // NewKeys returns a new instance of the Keys array with the provided capacity. func NewKeys(capacity int) Keys { - return make([]Key, 0, capacity) + return Keys{make([]Key, 0, capacity)} } // NewKeysWith is a helper method for returning a new keys array which includes the // the provided keys func NewKeysWith(key ...Key) Keys { - return append([]Key{}, key...) + return Keys{append([]Key{}, key...)} } // ================================== public methods ================================== func (k *Keys) Append(keys ...Key) { + ks := make([]Key, 0) for _, key := range keys { if key == nil && key.Raw() == nil { // don't track nil keys continue } - for _, kk := range *k { // skip duplicates + for _, kk := range k.keys { // skip duplicates if kk == key { return } } - *k = append(*k, key) + ks = append(ks, key) } + k.keys = append(k.keys, ks...) } func (k Keys) Capacity() int { - return cap(k) + return cap(k.keys) } func (k Keys) Length() int { - return len(k) + return len(k.keys) +} + +func (k Keys) ClearAll() { + k.keys = make([]Key, 0, len(k.keys)) } -func (k *Keys) ClearAll() { - *k = make([]Key, 0, len(*k)) +func (k Keys) Keys() []Key { + return k.keys } -func (k *Keys) Keys() []interface{} { +func (k Keys) RawKeys() []interface{} { result := make([]interface{}, 0, k.Length()) - for _, val := range *k { - result = append(result, val.Raw()) + for _, key := range k.keys { + result = append(result, key.Raw()) } return result } -func (k *Keys) StringKeys() []string { +func (k Keys) StringKeys() []string { result := make([]string, 0, k.Length()) - for _, val := range *k { - result = append(result, val.String()) + for _, key := range k.keys { + result = append(result, key.String()) } return result } -func (k *Keys) IsEmpty() bool { - return len(*k) == 0 +func (k Keys) IsEmpty() bool { + return len(k.keys) == 0 } diff --git a/result.go b/result.go index 6b0e18e..702d0dc 100644 --- a/result.go +++ b/result.go @@ -7,12 +7,12 @@ type Result struct { } // ResultMap maps each loaded elements Result against the elements unique identifier (Key) -type ResultMap map[Key]Result +type ResultMap map[string]Result // NewResultMap returns a new instance of the result map with the provided capacity. // Each value defaults to nil func NewResultMap(capacity int) ResultMap { - r := make(map[Key]Result, capacity) + r := make(map[string]Result, capacity) return r } @@ -20,7 +20,7 @@ func NewResultMap(capacity int) ResultMap { // Set adds the value to the to the result set. func (r ResultMap) Set(identifier Key, value Result) { - r[identifier] = value + r[identifier.String()] = value } // GetValue returns the value from the results for the provided key and true @@ -30,27 +30,19 @@ func (r ResultMap) GetValue(key Key) (Result, bool) { return Result{}, false } - result, ok := r[key] + result, ok := r[key.String()] return result, ok } -func (r ResultMap) GetValueForString(key StringKey) Result { +func (r ResultMap) GetValueForString(key string) Result { // No need to check ok, missing value from map[Any]interface{} is nil by default. return r[key] } -func (r ResultMap) Keys() []Key { - res := make([]Key, 0, len(r)) - for k, _ := range r { - res = append(res, k) - } - return res -} - -func (r ResultMap) StringKeys() []string { +func (r ResultMap) Keys() []string { res := make([]string, 0, len(r)) for k, _ := range r { - res = append(res, k.String()) + res = append(res, k) } return res } diff --git a/strategies/once/once_test.go b/strategies/once/once_test.go index 230a354..6f8dd5f 100644 --- a/strategies/once/once_test.go +++ b/strategies/once/once_test.go @@ -35,7 +35,7 @@ func getBatchFunction(cb func(), result dataloader.Result) dataloader.BatchFunct return func(ctx context.Context, keys dataloader.Keys) *dataloader.ResultMap { cb() m := dataloader.NewResultMap(1) - m.Set(keys.Keys()[0].(PrimaryKey).String(), result) + m.Set(keys.Keys()[0].(PrimaryKey), result) return &m } } @@ -302,7 +302,7 @@ func TestKeyHandling(t *testing.T) { for i := 0; i < keys.Length(); i++ { key := keys.Keys()[i].(PrimaryKey) if expectedResult[key] != "__skip__" { - m.Set(key.String(), dataloader.Result{Result: expectedResult[key], Err: nil}) + m.Set(key, dataloader.Result{Result: expectedResult[key], Err: nil}) } } return &m diff --git a/strategies/sozu/sozu.go b/strategies/sozu/sozu.go index 2111d14..5075928 100644 --- a/strategies/sozu/sozu.go +++ b/strategies/sozu/sozu.go @@ -317,7 +317,7 @@ func buildResultMap(keyArr []dataloader.Key, r dataloader.ResultMap) dataloader. for _, k := range keyArr { if val, ok := r.GetValue(k); ok { - results.Set(k.String(), val) + results.Set(k, val) } } diff --git a/strategies/sozu/sozu_test.go b/strategies/sozu/sozu_test.go index 183973c..d5e23bb 100644 --- a/strategies/sozu/sozu_test.go +++ b/strategies/sozu/sozu_test.go @@ -36,8 +36,8 @@ func getBatchFunction(cb func(dataloader.Keys), result string) dataloader.BatchF return func(ctx context.Context, keys dataloader.Keys) *dataloader.ResultMap { cb(keys) m := dataloader.NewResultMap(1) - for _, k := range keys.Keys() { - key := k.(PrimaryKey).String() + for _, k := range keys.RawKeys() { + key := k.(PrimaryKey) m.Set( key, dataloader.Result{ @@ -123,7 +123,7 @@ func TestLoadTimeoutTriggered(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) wg.Done() } @@ -206,7 +206,7 @@ func TestLoadManyTimeoutTriggered(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) wg.Done() } @@ -300,7 +300,7 @@ func TestLoadTriggered(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) wg.Done() } @@ -381,7 +381,7 @@ func TestLoadManyTriggered(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) wg.Done() } @@ -458,7 +458,7 @@ func TestLoadBlocked(t *testing.T) { expectedResult := "batch_on_timeout_load_many" cb := func(keys dataloader.Keys) { callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) } @@ -521,7 +521,7 @@ func TestLoadManyBlocked(t *testing.T) { expectedResult := "batch_on_timeout_load_many" cb := func(keys dataloader.Keys) { callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) } @@ -666,9 +666,9 @@ func TestKeyHandling(t *testing.T) { batch := func(ctx context.Context, keys dataloader.Keys) *dataloader.ResultMap { m := dataloader.NewResultMap(2) for i := 0; i < keys.Length(); i++ { - key := keys.Keys()[i].(PrimaryKey) + key := keys.RawKeys()[i].(PrimaryKey) if expectedResult[key] != "__skip__" { - m.Set(key.String(), dataloader.Result{Result: expectedResult[key], Err: nil}) + m.Set(key, dataloader.Result{Result: expectedResult[key], Err: nil}) } } return &m diff --git a/strategies/standard/standard.go b/strategies/standard/standard.go index 08ecf4e..9410f42 100644 --- a/strategies/standard/standard.go +++ b/strategies/standard/standard.go @@ -2,6 +2,7 @@ package standard import ( "context" + "fmt" "sync" "time" @@ -277,9 +278,11 @@ func buildResultMap(keyArr []dataloader.Key, r dataloader.ResultMap) dataloader. for _, k := range keyArr { if val, ok := r.GetValue(k); ok { - results.Set(k.String(), val) + results.Set(k, val) } } + fmt.Printf("r: %+v\nresults: %+v\n", r, results) + return results } diff --git a/strategies/standard/standard_test.go b/strategies/standard/standard_test.go index 10b6971..8eb33ab 100644 --- a/strategies/standard/standard_test.go +++ b/strategies/standard/standard_test.go @@ -37,7 +37,7 @@ func getBatchFunction(cb func(dataloader.Keys), result string) dataloader.BatchF cb(keys) m := dataloader.NewResultMap(1) for _, k := range keys.Keys() { - key := k.(PrimaryKey).String() + key := k.(PrimaryKey) m.Set( key, dataloader.Result{ @@ -123,7 +123,7 @@ func TestLoadNoTimeout(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) wg.Done() } @@ -204,7 +204,7 @@ func TestLoadManyNoTimeout(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() close(closeChan) wg.Done() } @@ -291,7 +291,7 @@ func TestLoadTimeout(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() if callCount == 2 { close(closeChan) } @@ -389,7 +389,7 @@ func TestLoadManyTimeout(t *testing.T) { cb := func(keys dataloader.Keys) { blockWG.Wait() callCount += 1 - k = keys.Keys() + k = keys.RawKeys() if callCount == 2 { close(closeChan) } @@ -556,7 +556,7 @@ func TestKeyHandling(t *testing.T) { for i := 0; i < keys.Length(); i++ { key := keys.Keys()[i].(PrimaryKey) if expectedResult[key] != "__skip__" { - m.Set(key.String(), dataloader.Result{Result: expectedResult[key], Err: nil}) + m.Set(key, dataloader.Result{Result: expectedResult[key], Err: nil}) } } return &m diff --git a/trace.go b/trace.go index 958c7bf..b61e431 100644 --- a/trace.go +++ b/trace.go @@ -82,7 +82,7 @@ func (*openTracer) Batch(ctx context.Context) (context.Context, BatchFinishFunc) span, spanCtx := opentracing.StartSpanFromContext(ctx, "Dataloader: batch") return spanCtx, func(r ResultMap) { - span.SetTag("keys", fmt.Sprintf("[%s]", strings.Join(r.StringKeys(), ", "))) + span.SetTag("keys", fmt.Sprintf("[%s]", strings.Join(r.Keys(), ", "))) span.Finish() } } From d50781b39eacd09400d523a403bde7bc3e5586ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Fri, 29 Nov 2019 01:32:42 -0500 Subject: [PATCH 05/11] Fix bug with nil keys --- key.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/key.go b/key.go index dd0e86a..03577a7 100644 --- a/key.go +++ b/key.go @@ -43,7 +43,7 @@ func NewKeysWith(key ...Key) Keys { func (k *Keys) Append(keys ...Key) { ks := make([]Key, 0) for _, key := range keys { - if key == nil && key.Raw() == nil { // don't track nil keys + if key == nil || key.Raw() == nil { // don't track nil keys continue } for _, kk := range k.keys { // skip duplicates From 35c84269ee22ed7a141c6f504c3476748005b9c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Fri, 29 Nov 2019 01:35:47 -0500 Subject: [PATCH 06/11] Enhance cache handling in dataloader.LoadMany() --- dataloader.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/dataloader.go b/dataloader.go index 2b6f97c..cd980bc 100644 --- a/dataloader.go +++ b/dataloader.go @@ -153,21 +153,26 @@ func (d *dataloader) Load(ogCtx context.Context, key Key) Thunk { func (d *dataloader) LoadMany(ogCtx context.Context, keyArr ...Key) ThunkMany { ctx, finish := d.tracer.LoadMany(ogCtx, keyArr) - if r, ok := d.cache.GetResultMap(ctx, keyArr...); ok { - d.logger.Logf("cache hit for: %d", keyArr) - d.strategy.LoadNoOp(ctx) - return func() ResultMap { - finish(r) - - return r + var cached, missed = ResultMap{}, []Key{} + for _, key := range keyArr { + if r, ok := d.cache.GetResult(ctx, key); ok { + d.logger.Logf("cache hit for: %d", key) + d.strategy.LoadNoOp(ctx) + cached[key.String()] = r + } else { + missed = append(missed, key) } } - thunkMany := d.strategy.LoadMany(ctx, keyArr...) + thunkMany := d.strategy.LoadMany(ctx, missed...) return func() ResultMap { + cached := cached result := thunkMany() d.cache.SetResultMap(ctx, result) + for k, v := range cached { + result[k] = v + } finish(result) return result From 307fc121f043a40a8ea2d6bf13884841c294c8a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Fri, 29 Nov 2019 23:59:53 -0500 Subject: [PATCH 07/11] Fix bug with Keys.RawKeys() + Keys.StringKeys() generation --- key.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/key.go b/key.go index 03577a7..7742530 100644 --- a/key.go +++ b/key.go @@ -73,17 +73,17 @@ func (k Keys) Keys() []Key { } func (k Keys) RawKeys() []interface{} { - result := make([]interface{}, 0, k.Length()) - for _, key := range k.keys { - result = append(result, key.Raw()) + result := make([]interface{}, k.Length()) + for i := 0; i < len(k.keys); i++ { + result[i] = k.keys[i].Raw() } return result } func (k Keys) StringKeys() []string { - result := make([]string, 0, k.Length()) - for _, key := range k.keys { - result = append(result, key.String()) + result := make([]string, k.Length()) + for i := 0; i < len(k.keys); i++ { + result[i] = k.keys[i].String() } return result } From 665722381d9f79633a921a37f8d5a8d012f453fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Sun, 1 Dec 2019 19:32:42 -0500 Subject: [PATCH 08/11] Fix skip duplicates on Keys.Append(...) --- key.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/key.go b/key.go index 7742530..20bb596 100644 --- a/key.go +++ b/key.go @@ -41,19 +41,17 @@ func NewKeysWith(key ...Key) Keys { // ================================== public methods ================================== func (k *Keys) Append(keys ...Key) { - ks := make([]Key, 0) - for _, key := range keys { - if key == nil || key.Raw() == nil { // don't track nil keys - continue - } - for _, kk := range k.keys { // skip duplicates - if kk == key { - return + appendIfMissing := func(keys []Key, k Key) []Key { + for _, key := range keys { + if key.String() == k.String() { + return keys } } - ks = append(ks, key) + return append(keys, k) + } + for _, key := range keys { + k.keys = appendIfMissing(k.keys, key) } - k.keys = append(k.keys, ks...) } func (k Keys) Capacity() int { From 45478e5c5e35eb22c79bcc736e7159c10a551dad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Mon, 2 Dec 2019 19:41:22 -0500 Subject: [PATCH 09/11] Revert Keys to original state --- key.go | 81 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 32 deletions(-) diff --git a/key.go b/key.go index 20bb596..6c4678f 100644 --- a/key.go +++ b/key.go @@ -12,11 +12,6 @@ type Key interface { Raw() interface{} } -// Keys wraps an array of keys and contains accessor methods -type Keys struct { - keys []Key -} - type StringKey string func (k StringKey) String() string { @@ -27,65 +22,87 @@ func (k StringKey) Raw() interface{} { return k } +// Keys wraps an array of keys and contains accessor methods +type Keys interface { + Append(...Key) + Capacity() int + Length() int + ClearAll() + // Keys returns a an array of unique results after calling Raw on each key + Keys() []interface{} + StringKeys() []string + IsEmpty() bool +} + +type keys struct { + keys []Key +} + // NewKeys returns a new instance of the Keys array with the provided capacity. func NewKeys(capacity int) Keys { - return Keys{make([]Key, 0, capacity)} + return &keys{ + keys: make([]Key, 0, capacity), + } } // NewKeysWith is a helper method for returning a new keys array which includes the // the provided keys func NewKeysWith(key ...Key) Keys { - return Keys{append([]Key{}, key...)} + return &keys{ + keys: key, + } } // ================================== public methods ================================== -func (k *Keys) Append(keys ...Key) { - appendIfMissing := func(keys []Key, k Key) []Key { - for _, key := range keys { - if key.String() == k.String() { - return keys - } - } - return append(keys, k) - } +func (k *keys) Append(keys ...Key) { for _, key := range keys { - k.keys = appendIfMissing(k.keys, key) + if key != nil && key.Raw() != nil { // don't track nil keys + k.keys = append(k.keys, key) + } } } -func (k Keys) Capacity() int { +func (k *keys) Capacity() int { return cap(k.keys) } -func (k Keys) Length() int { +func (k *keys) Length() int { return len(k.keys) } -func (k Keys) ClearAll() { +func (k *keys) ClearAll() { k.keys = make([]Key, 0, len(k.keys)) } -func (k Keys) Keys() []Key { - return k.keys -} +func (k *keys) Keys() []interface{} { + result := make([]interface{}, 0, k.Length()) + temp := make(map[Key]bool, k.Length()) -func (k Keys) RawKeys() []interface{} { - result := make([]interface{}, k.Length()) - for i := 0; i < len(k.keys); i++ { - result[i] = k.keys[i].Raw() + for _, val := range k.keys { + if _, ok := temp[val]; !ok { + temp[val] = true + result = append(result, val.Raw()) + } } + return result } -func (k Keys) StringKeys() []string { - result := make([]string, k.Length()) - for i := 0; i < len(k.keys); i++ { - result[i] = k.keys[i].String() +func (k *keys) StringKeys() []string { + result := make([]string, 0, k.Length()) + temp := make(map[Key]bool, k.Length()) + + for _, val := range k.keys { + if _, ok := temp[val]; !ok { + temp[val] = true + result = append(result, val.String()) + } } + return result } -func (k Keys) IsEmpty() bool { +func (k *keys) IsEmpty() bool { return len(k.keys) == 0 } From 4cd32074af241986a36df1d9de17b07ae187b4dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Mon, 2 Dec 2019 22:07:22 -0500 Subject: [PATCH 10/11] Fix bug with dataloader.LoadMany() when all keys are served from cache --- dataloader.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dataloader.go b/dataloader.go index cd980bc..e7d8493 100644 --- a/dataloader.go +++ b/dataloader.go @@ -164,6 +164,13 @@ func (d *dataloader) LoadMany(ogCtx context.Context, keyArr ...Key) ThunkMany { } } + if len(missed) == 0 { + finish(cached) + return func() ResultMap { + return cached + } + } + thunkMany := d.strategy.LoadMany(ctx, missed...) return func() ResultMap { cached := cached From 1a1ce28765b5511941ec0a4a088801022e48e08f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Jak=C3=B3n?= Date: Mon, 2 Dec 2019 22:25:53 -0500 Subject: [PATCH 11/11] Move tracer finish function into wrapped ThunkMany result --- dataloader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dataloader.go b/dataloader.go index e7d8493..8a7fda7 100644 --- a/dataloader.go +++ b/dataloader.go @@ -165,8 +165,8 @@ func (d *dataloader) LoadMany(ogCtx context.Context, keyArr ...Key) ThunkMany { } if len(missed) == 0 { - finish(cached) return func() ResultMap { + finish(cached) return cached } }