From 1828cbb573e93f331d8da4ee387f0b2eabc1efee Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 13 Jan 2022 09:39:33 -0600 Subject: [PATCH 1/2] cache: avoid repeated map lookups Instead of repeatedly accessing multi-level maps each time through a loop, just access the top-level map one time. Benchmarks show slight improvement in averages over 5 test runs of of 10,000 indexed items on an Intel i7-10610U CPU @ 1.80GHz. Test: NewCreate OldCreate NewUpdate OldUpdate NewDelete OldDelete NewExist OldExist ---------------------------------------------------------------------------------- 29460494 30486747 54120726 56350494 39815101 40666383 8778104 10049136 30636038 30757173 51851904 53903189 39999444 40050579 8583703 8908518 30391577 31052420 53183252 54717886 39888502 40389043 8923286 8814861 29889240 32371633 54644802 55370656 42198881 41440149 8953393 8783270 30250150 31542984 53576849 54834028 40063272 44855216 8892491 8913675 31934634 35215783 54054254 55033390 39934407 40392941 8750578 8912334 31805916 36200180 55738671 54962591 40013275 44188996 8754013 8788007 32186922 32530255 57006004 53838629 41504985 41843850 8886529 8885310 32436142 32350717 56428116 56927858 51300396 40936654 8799192 9117470 31403532 32358590 53220055 55925679 40066883 40801672 8815105 9575259 30843257 31280280 51717953 57669164 39556915 41193336 8621941 9597315 30891843 32596735 52627556 55359623 38737549 40536766 9049546 9800374 31589489 34062320 53301421 57017124 40324738 45095575 8788486 8848887 31784935 33273792 54904916 56347236 41777111 41707506 8658814 9803214 31939747 32015268 54239797 56389970 41807871 43295242 9046912 8880392 ---------------------------------------------------------------------------------- Avg: 31162927 32539658 54041085 55643167 41132622 41826260 8820139 9178534 Incr: +4.23% +2.88% +1.66% +3.90% Signed-off-by: Dan Williams --- cache/cache.go | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index 8a3c86e8..9dcb2907 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -127,12 +127,12 @@ func (r *RowCache) RowByModel(m model.Model) model.Model { if uuid.(string) != "" { return r.rowByUUID(uuid.(string)) } - for index := range r.indexes { + for index, vals := range r.indexes { val, err := valueFromIndex(info, index) if err != nil { continue } - if uuid, ok := r.indexes[index][val]; ok { + if uuid, ok := vals[val]; ok { return r.rowByUUID(uuid) } } @@ -154,13 +154,13 @@ func (r *RowCache) Create(uuid string, m model.Model, checkIndexes bool) error { return err } newIndexes := newColumnToValue(r.dbModel.Schema.Table(r.name).Indexes) - for index := range r.indexes { + for index, vals := range r.indexes { val, err := valueFromIndex(info, index) if err != nil { return err } - if existing, ok := r.indexes[index][val]; ok && checkIndexes { + if existing, ok := vals[val]; ok && checkIndexes { return NewIndexExistsError(r.name, val, string(index), uuid, existing) } @@ -169,8 +169,9 @@ func (r *RowCache) Create(uuid string, m model.Model, checkIndexes bool) error { // write indexes for k1, v1 := range newIndexes { + vals := r.indexes[k1] for k2, v2 := range v1 { - r.indexes[k1][k2] = v2 + vals[k2] = v2 } } r.cache[uuid] = model.Clone(m) @@ -197,7 +198,7 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { newIndexes := newColumnToValue(indexes) oldIndexes := newColumnToValue(indexes) var errs []error - for index := range r.indexes { + for index, vals := range r.indexes { var err error oldVal, err := valueFromIndex(oldInfo, index) if err != nil { @@ -215,7 +216,7 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { // old and new values are NOT the same // check that there are no conflicts - if conflict, ok := r.indexes[index][newVal]; ok && checkIndexes && conflict != uuid { + if conflict, ok := vals[newVal]; ok && checkIndexes && conflict != uuid { errs = append(errs, NewIndexExistsError( r.name, newVal, @@ -233,14 +234,16 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { } // write indexes for k1, v1 := range newIndexes { + vals := r.indexes[k1] for k2, v2 := range v1 { - r.indexes[k1][k2] = v2 + vals[k2] = v2 } } // delete old indexes for k1, v1 := range oldIndexes { + vals := r.indexes[k1] for k2 := range v1 { - delete(r.indexes[k1], k2) + delete(vals, k2) } } r.cache[uuid] = model.Clone(m) @@ -256,12 +259,12 @@ func (r *RowCache) IndexExists(row model.Model) error { if err != nil { return nil } - for index := range r.indexes { + for index, vals := range r.indexes { val, err := valueFromIndex(info, index) if err != nil { continue } - if existing, ok := r.indexes[index][val]; ok && existing != uuid.(string) { + if existing, ok := vals[val]; ok && existing != uuid.(string) { return NewIndexExistsError( r.name, val, @@ -286,7 +289,7 @@ func (r *RowCache) Delete(uuid string) error { if err != nil { return err } - for index := range r.indexes { + for index, vals := range r.indexes { oldVal, err := valueFromIndex(oldInfo, index) if err != nil { return err @@ -294,8 +297,8 @@ func (r *RowCache) Delete(uuid string) error { // only remove the index if it is pointing to this uuid // otherwise we can cause a consistency issue if we've processed // updates out of order - if r.indexes[index][oldVal] == uuid { - delete(r.indexes[index], oldVal) + if vals[oldVal] == uuid { + delete(vals, oldVal) } } delete(r.cache, uuid) @@ -470,8 +473,9 @@ func NewTableCache(dbModel model.DatabaseModel, data Data, logger *logr.Logger) if _, ok := dbModel.Schema.Tables[table]; !ok { return nil, fmt.Errorf("table %s is not in schema", table) } + rowCache := cache[table] for uuid, row := range rowData { - if err := cache[table].Create(uuid, row, true); err != nil { + if err := rowCache.Create(uuid, row, true); err != nil { return nil, err } } From 52ebc9c6c7e4532ceed1b5ff532c6dece7afaa1a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 13 Jan 2022 11:37:51 -0600 Subject: [PATCH 2/2] cache: add benchmarks for cache operations Signed-off-by: Dan Williams --- cache/cache_test.go | 74 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) diff --git a/cache/cache_test.go b/cache/cache_test.go index d7ae231f..a4124376 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -2,6 +2,7 @@ package cache import ( "encoding/json" + "fmt" "reflect" "testing" @@ -1200,7 +1201,7 @@ func TestIndex(t *testing.T) { }) } -func TestTableCacheRowByModelSingleIndex(t *testing.T) { +func setupRowByModelSingleIndex(t require.TestingT) (*testModel, *TableCache) { var schema ovsdb.DatabaseSchema db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) @@ -1214,8 +1215,8 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { "type": "string" }, "bar": { - "type": "string" - } + "type": "string" + } } } } @@ -1234,6 +1235,12 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { tc, err := NewTableCache(dbModel, testData, nil) require.NoError(t, err) + return myFoo, tc +} + +func TestTableCacheRowByModelSingleIndex(t *testing.T) { + myFoo, tc := setupRowByModelSingleIndex(t) + t.Run("get foo by index", func(t *testing.T) { foo := tc.Table("Open_vSwitch").RowByModel(&testModel{Foo: "foo"}) assert.NotNil(t, foo) @@ -1251,6 +1258,67 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { }) } +func benchmarkDoCreate(b *testing.B, numRows int) *RowCache { + _, tc := setupRowByModelSingleIndex(b) + + rc := tc.Table("Open_vSwitch") + for i := 0; i < numRows; i++ { + uuid := fmt.Sprintf("%d", i) + model := &testModel{Foo: uuid} + err := rc.Create(uuid, model, true) + require.NoError(b, err) + } + + return rc +} + +const numRows int = 10000 + +func BenchmarkSingleIndexCreate(b *testing.B) { + for n := 0; n < b.N; n++ { + _ = benchmarkDoCreate(b, numRows) + } +} + +func BenchmarkSingleIndexUpdate(b *testing.B) { + rc := benchmarkDoCreate(b, numRows) + + b.ResetTimer() + for n := 0; n < b.N; n++ { + for i := 0; i < numRows; i++ { + uuid := fmt.Sprintf("%d", i) + model := &testModel{Foo: fmt.Sprintf("%d-%d", n, i)} + err := rc.Update(uuid, model, true) + require.NoError(b, err) + } + } +} + +func BenchmarkSingleIndexDelete(b *testing.B) { + for n := 0; n < b.N; n++ { + rc := benchmarkDoCreate(b, numRows) + for i := 0; i < numRows; i++ { + uuid := fmt.Sprintf("%d", i) + err := rc.Delete(uuid) + require.NoError(b, err) + } + } +} + +func BenchmarkIndexExists(b *testing.B) { + rc := benchmarkDoCreate(b, numRows) + + b.ResetTimer() + for n := 0; n < b.N; n++ { + for i := 0; i < numRows; i++ { + uuid := fmt.Sprintf("%d", i) + model := &testModel{UUID: uuid, Foo: uuid} + err := rc.IndexExists(model) + require.NoError(b, err) + } + } +} + func TestTableCacheRowByModelTwoIndexes(t *testing.T) { var schema ovsdb.DatabaseSchema db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}})