From 12bf65bbb93072e33b96d1ae0bc8272f1f83ab8c Mon Sep 17 00:00:00 2001 From: Josh Baker Date: Fri, 30 Dec 2016 14:18:28 -0700 Subject: [PATCH 1/4] wip --- controller/collection/collection.go | 40 ++++++++++++++++-------- controller/collection/collection_test.go | 20 ++++++++++++ 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/controller/collection/collection.go b/controller/collection/collection.go index 0b6235cc..eb51f74a 100644 --- a/controller/collection/collection.go +++ b/controller/collection/collection.go @@ -101,9 +101,8 @@ func (c *Collection) Bounds() (minX, minY, minZ, maxX, maxY, maxZ float64) { // The fields argument is optional. // The return values are the old object, the old fields, and the new fields func (c *Collection) ReplaceOrInsert(id string, obj geojson.Object, fields []string, values []float64) (oldObject geojson.Object, oldFields []float64, newFields []float64) { - oldItem, ok := c.remove(id) - nitem := c.insert(id, obj) - if ok { + nitem, oldItem := c.insertOrReplace(id, obj) + if oldItem != nil { oldObject = oldItem.object oldFields = oldItem.fields nitem.fields = oldFields @@ -143,19 +142,34 @@ func (c *Collection) remove(id string) (item *itemT, ok bool) { return item, true } -func (c *Collection) insert(id string, obj geojson.Object) (item *itemT) { +func (c *Collection) insertOrReplace(id string, obj geojson.Object) (item, prev *itemT) { item = &itemT{id: id, object: obj} - if obj.IsGeometry() { - c.index.Insert(item) - c.objects++ + old := c.items.ReplaceOrInsert(item) + if old != nil { + prev = old.(*itemT) + if item.object.IsGeometry() { + c.index.Remove(prev) + c.index.Insert(item) + } else { + c.values.ReplaceOrInsert(item) + } + c.weight -= len(prev.fields) * 8 + c.weight -= prev.object.Weight() + len(prev.id) + c.points -= prev.object.PositionCount() + c.weight += obj.Weight() + len(id) + c.points += obj.PositionCount() } else { - c.values.ReplaceOrInsert(item) - c.nobjects++ + if obj.IsGeometry() { + c.index.Insert(item) + c.objects++ + } else { + c.values.ReplaceOrInsert(item) + c.nobjects++ + } + c.weight += obj.Weight() + len(id) + c.points += obj.PositionCount() } - c.items.ReplaceOrInsert(item) - c.weight += obj.Weight() + len(id) - c.points += obj.PositionCount() - return item + return item, prev } // Remove removes an object and returns it. diff --git a/controller/collection/collection_test.go b/controller/collection/collection_test.go index 8f74a1e7..63743e87 100644 --- a/controller/collection/collection_test.go +++ b/controller/collection/collection_test.go @@ -1,9 +1,11 @@ package collection import ( + "fmt" "math/rand" "strconv" "testing" + "time" "github.com/tidwall/tile38/geojson" ) @@ -87,3 +89,21 @@ func TestManyCollections(t *testing.T) { return true }) } + +func BenchmarkInsert(t *testing.B) { + rand.Seed(time.Now().UnixNano()) + ids := make([]string, t.N) + points := make([]geojson.Object, t.N) + for i := 0; i < t.N; i++ { + points[i] = geojson.SimplePoint{ + Y: rand.Float64()*180 - 90, + X: rand.Float64()*360 - 180, + } + ids[i] = fmt.Sprintf("%d", i) + } + col := New() + t.ResetTimer() + for i := 0; i < t.N; i++ { + col.ReplaceOrInsert(ids[i], points[i], nil, nil) + } +} From d6ca25d14b71803c2b66174356f88fd9ea599c08 Mon Sep 17 00:00:00 2001 From: Josh Baker Date: Fri, 30 Dec 2016 18:12:18 -0700 Subject: [PATCH 2/4] updated collection ReplaceOrInsert 10% bump --- controller/collection/collection.go | 94 ++++++++++++++++++----------- tests/keys_test.go | 16 +++++ tests/tests_test.go | 5 +- 3 files changed, 78 insertions(+), 37 deletions(-) diff --git a/controller/collection/collection.go b/controller/collection/collection.go index eb51f74a..d2c72c3e 100644 --- a/controller/collection/collection.go +++ b/controller/collection/collection.go @@ -101,26 +101,63 @@ func (c *Collection) Bounds() (minX, minY, minZ, maxX, maxY, maxZ float64) { // The fields argument is optional. // The return values are the old object, the old fields, and the new fields func (c *Collection) ReplaceOrInsert(id string, obj geojson.Object, fields []string, values []float64) (oldObject geojson.Object, oldFields []float64, newFields []float64) { - nitem, oldItem := c.insertOrReplace(id, obj) - if oldItem != nil { + var oldItem *itemT + var newItem *itemT = &itemT{id: id, object: obj} + // add the new item to main btree and remove the old one if needed + oldItemPtr := c.items.ReplaceOrInsert(newItem) + if oldItemPtr != nil { + // the old item was removed, now let's remove from the rtree + // or strings tree. + oldItem = oldItemPtr.(*itemT) + if obj.IsGeometry() { + // geometry + c.index.Remove(oldItem) + c.objects-- + } else { + // string + c.values.Delete(oldItem) + c.nobjects-- + } + // decrement the point count + c.points -= oldItem.object.PositionCount() + + // decrement the weights + c.weight -= len(oldItem.fields) * 8 + c.weight -= oldItem.object.Weight() + len(oldItem.id) + + // references oldObject = oldItem.object oldFields = oldItem.fields - nitem.fields = oldFields - c.weight += len(nitem.fields) * 8 + newItem.fields = oldFields } - if fields == nil && len(values) > 0 { - // directly set the field values, update weight - c.weight -= len(nitem.fields) * 8 - nitem.fields = values - c.weight += len(nitem.fields) * 8 + // insert the new item into the rtree or strings tree. + if obj.IsGeometry() { + c.index.Insert(newItem) + c.objects++ + } else { + c.values.ReplaceOrInsert(newItem) + c.nobjects++ + } + // increment the point count + c.points += obj.PositionCount() + // add the new weights + c.weight += len(newItem.fields) * 8 + c.weight += obj.Weight() + len(id) + if fields == nil { + if len(values) > 0 { + // directly set the field values, update weight + c.weight -= len(newItem.fields) * 8 + newItem.fields = values + c.weight += len(newItem.fields) * 8 + } } else { // map field name to value for i, field := range fields { - c.setField(nitem, field, values[i]) + c.setField(newItem, field, values[i]) } } - return oldObject, oldFields, nitem.fields + return oldObject, oldFields, newItem.fields } func (c *Collection) remove(id string) (item *itemT, ok bool) { @@ -142,34 +179,19 @@ func (c *Collection) remove(id string) (item *itemT, ok bool) { return item, true } -func (c *Collection) insertOrReplace(id string, obj geojson.Object) (item, prev *itemT) { +func (c *Collection) insert(id string, obj geojson.Object) (item *itemT) { item = &itemT{id: id, object: obj} - old := c.items.ReplaceOrInsert(item) - if old != nil { - prev = old.(*itemT) - if item.object.IsGeometry() { - c.index.Remove(prev) - c.index.Insert(item) - } else { - c.values.ReplaceOrInsert(item) - } - c.weight -= len(prev.fields) * 8 - c.weight -= prev.object.Weight() + len(prev.id) - c.points -= prev.object.PositionCount() - c.weight += obj.Weight() + len(id) - c.points += obj.PositionCount() + if obj.IsGeometry() { + c.index.Insert(item) + c.objects++ } else { - if obj.IsGeometry() { - c.index.Insert(item) - c.objects++ - } else { - c.values.ReplaceOrInsert(item) - c.nobjects++ - } - c.weight += obj.Weight() + len(id) - c.points += obj.PositionCount() + c.values.ReplaceOrInsert(item) + c.nobjects++ } - return item, prev + c.items.ReplaceOrInsert(item) + c.weight += obj.Weight() + len(id) + c.points += obj.PositionCount() + return item } // Remove removes an object and returns it. diff --git a/tests/keys_test.go b/tests/keys_test.go index 09259fb4..8e2085be 100644 --- a/tests/keys_test.go +++ b/tests/keys_test.go @@ -29,6 +29,7 @@ func subTestKeys(t *testing.T, mc *mockServer) { runStep(t, mc, "TTL", keys_TTL_test) runStep(t, mc, "SET EX", keys_SET_EX_test) runStep(t, mc, "PDEL", keys_PDEL_test) + runStep(t, mc, "FIELDS", keys_FIELDS_test) } func keys_BOUNDS_test(mc *mockServer) error { @@ -325,6 +326,21 @@ func keys_SET_EX_test(mc *mockServer) (err error) { return nil } +func keys_FIELDS_test(mc *mockServer) error { + return mc.DoBatch([][]interface{}{ + {"SET", "mykey", "myid1a", "FIELD", "a", 1, "POINT", 33, -115}, {"OK"}, + {"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1]]`}, + {"SET", "mykey", "myid1a", "FIELD", "a", "a", "POINT", 33, -115}, {"ERR invalid argument 'a'"}, + {"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1]]`}, + {"SET", "mykey", "myid1a", "FIELD", "a", 1, "FIELD", "b", 2, "POINT", 33, -115}, {"OK"}, + {"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2]]`}, + {"SET", "mykey", "myid1a", "FIELD", "b", 2, "POINT", 33, -115}, {"OK"}, + {"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2]]`}, + {"SET", "mykey", "myid1a", "FIELD", "b", 2, "FIELD", "a", "1", "FIELD", "c", 3, "POINT", 33, -115}, {"OK"}, + {"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2 c 3]]`}, + }) +} + func keys_PDEL_test(mc *mockServer) error { return mc.DoBatch([][]interface{}{ {"SET", "mykey", "myid1a", "POINT", 33, -115}, {"OK"}, diff --git a/tests/tests_test.go b/tests/tests_test.go index 266dfdb9..e38fa37e 100644 --- a/tests/tests_test.go +++ b/tests/tests_test.go @@ -57,7 +57,10 @@ func runStep(t *testing.T, mc *mockServer, name string, step func(mc *mockServer mc.ResetConn() defer mc.ResetConn() // clear the database so the test is consistent - if err := mc.DoBatch([][]interface{}{{"FLUSHDB"}, {"OK"}}); err != nil { + if err := mc.DoBatch([][]interface{}{ + {"OUTPUT", "resp"}, {"OK"}, + {"FLUSHDB"}, {"OK"}, + }); err != nil { return err } if err := step(mc); err != nil { From b9e61777e6c9f2c347871d19bb3b5bca5fac3f5a Mon Sep 17 00:00:00 2001 From: Josh Baker Date: Sat, 31 Dec 2016 09:29:02 -0700 Subject: [PATCH 3/4] moved fields to a collection map --- controller/collection/collection.go | 145 ++++++++++++----------- controller/collection/collection_test.go | 95 +++++++++++++-- 2 files changed, 161 insertions(+), 79 deletions(-) diff --git a/controller/collection/collection.go b/controller/collection/collection.go index d2c72c3e..dca07d9b 100644 --- a/controller/collection/collection.go +++ b/controller/collection/collection.go @@ -14,7 +14,6 @@ const ( type itemT struct { id string object geojson.Object - fields []float64 } func (i *itemT) Less(item btree.Item, ctx interface{}) bool { @@ -48,14 +47,15 @@ func (i *itemT) Point() (x, y, z float64) { // Collection represents a collection of geojson objects. type Collection struct { - items *btree.BTree // items sorted by keys - values *btree.BTree // items sorted by value+key - index *index.Index // items geospatially indexed - fieldMap map[string]int - weight int - points int - objects int // geometry count - nobjects int // non-geometry count + items *btree.BTree // items sorted by keys + values *btree.BTree // items sorted by value+key + index *index.Index // items geospatially indexed + fieldMap map[string]int + fieldValues map[string][]float64 + weight int + points int + objects int // geometry count + nobjects int // non-geometry count } var counter uint64 @@ -64,13 +64,32 @@ var counter uint64 func New() *Collection { col := &Collection{ index: index.New(), - items: btree.New(48, idOrdered), - values: btree.New(48, valueOrdered), + items: btree.New(128, idOrdered), + values: btree.New(128, valueOrdered), fieldMap: make(map[string]int), } return col } +func (c *Collection) setFieldValues(id string, values []float64) { + if c.fieldValues == nil { + c.fieldValues = make(map[string][]float64) + } + c.fieldValues[id] = values +} + +func (c *Collection) getFieldValues(id string) (values []float64) { + if c.fieldValues == nil { + return nil + } + return c.fieldValues[id] +} +func (c *Collection) deleteFieldValues(id string) { + if c.fieldValues != nil { + delete(c.fieldValues, id) + } +} + // Count returns the number of objects in collection. func (c *Collection) Count() int { return c.objects + c.nobjects @@ -122,13 +141,13 @@ func (c *Collection) ReplaceOrInsert(id string, obj geojson.Object, fields []str c.points -= oldItem.object.PositionCount() // decrement the weights - c.weight -= len(oldItem.fields) * 8 + c.weight -= len(c.getFieldValues(id)) * 8 c.weight -= oldItem.object.Weight() + len(oldItem.id) // references oldObject = oldItem.object - oldFields = oldItem.fields - newItem.fields = oldFields + oldFields = c.getFieldValues(id) + newFields = oldFields } // insert the new item into the rtree or strings tree. if obj.IsGeometry() { @@ -142,30 +161,37 @@ func (c *Collection) ReplaceOrInsert(id string, obj geojson.Object, fields []str c.points += obj.PositionCount() // add the new weights - c.weight += len(newItem.fields) * 8 + c.weight += len(newFields) * 8 c.weight += obj.Weight() + len(id) if fields == nil { if len(values) > 0 { // directly set the field values, update weight - c.weight -= len(newItem.fields) * 8 - newItem.fields = values - c.weight += len(newItem.fields) * 8 + c.weight -= len(newFields) * 8 + newFields = values + c.setFieldValues(id, newFields) + c.weight += len(newFields) * 8 } } else { + if len(fields) == 0 { + panic("if fields is empty, make it nil") + } // map field name to value for i, field := range fields { c.setField(newItem, field, values[i]) } + newFields = c.getFieldValues(id) } - return oldObject, oldFields, newItem.fields + return oldObject, oldFields, newFields } -func (c *Collection) remove(id string) (item *itemT, ok bool) { +// Remove removes an object and returns it. +// If the object does not exist then the 'ok' return value will be false. +func (c *Collection) Remove(id string) (obj geojson.Object, fields []float64, ok bool) { i := c.items.Delete(&itemT{id: id}) if i == nil { - return nil, false + return nil, nil, false } - item = i.(*itemT) + item := i.(*itemT) if item.object.IsGeometry() { c.index.Remove(item) c.objects-- @@ -173,50 +199,23 @@ func (c *Collection) remove(id string) (item *itemT, ok bool) { c.values.Delete(item) c.nobjects-- } - c.weight -= len(item.fields) * 8 + fields = c.getFieldValues(id) + c.deleteFieldValues(id) + c.weight -= len(fields) * 8 c.weight -= item.object.Weight() + len(item.id) c.points -= item.object.PositionCount() - return item, true -} - -func (c *Collection) insert(id string, obj geojson.Object) (item *itemT) { - item = &itemT{id: id, object: obj} - if obj.IsGeometry() { - c.index.Insert(item) - c.objects++ - } else { - c.values.ReplaceOrInsert(item) - c.nobjects++ - } - c.items.ReplaceOrInsert(item) - c.weight += obj.Weight() + len(id) - c.points += obj.PositionCount() - return item -} - -// Remove removes an object and returns it. -// If the object does not exist then the 'ok' return value will be false. -func (c *Collection) Remove(id string) (obj geojson.Object, fields []float64, ok bool) { - item, ok := c.remove(id) - if !ok { - return nil, nil, false - } - return item.object, item.fields, true -} - -func (c *Collection) get(id string) (obj geojson.Object, fields []float64, ok bool) { - i := c.items.Get(&itemT{id: id}) - if i == nil { - return nil, nil, false - } - item := i.(*itemT) - return item.object, item.fields, true + return item.object, fields, true } // Get returns an object. // If the object does not exist then the 'ok' return value will be false. func (c *Collection) Get(id string) (obj geojson.Object, fields []float64, ok bool) { - return c.get(id) + i := c.items.Get(&itemT{id: id}) + if i == nil { + return nil, nil, false + } + item := i.(*itemT) + return item.object, c.getFieldValues(id), true } // SetField set a field value for an object and returns that object. @@ -229,7 +228,7 @@ func (c *Collection) SetField(id, field string, value float64) (obj geojson.Obje } item := i.(*itemT) updated = c.setField(item, field, value) - return item.object, item.fields, updated, true + return item.object, c.getFieldValues(id), updated, true } func (c *Collection) setField(item *itemT, field string, value float64) (updated bool) { @@ -238,13 +237,15 @@ func (c *Collection) setField(item *itemT, field string, value float64) (updated idx = len(c.fieldMap) c.fieldMap[field] = idx } - c.weight -= len(item.fields) * 8 - for idx >= len(item.fields) { - item.fields = append(item.fields, 0) + fields := c.getFieldValues(item.id) + c.weight -= len(fields) * 8 + for idx >= len(fields) { + fields = append(fields, 0) } - c.weight += len(item.fields) * 8 - ovalue := item.fields[idx] - item.fields[idx] = value + c.weight += len(fields) * 8 + ovalue := fields[idx] + fields[idx] = value + c.setFieldValues(item.id, fields) return ovalue != value } @@ -271,7 +272,7 @@ func (c *Collection) Scan(cursor uint64, desc bool, iter := func(item btree.Item) bool { if i >= cursor { iitm := item.(*itemT) - active = iterator(iitm.id, iitm.object, iitm.fields) + active = iterator(iitm.id, iitm.object, c.getFieldValues(iitm.id)) } i++ return active @@ -293,7 +294,7 @@ func (c *Collection) ScanRange(cursor uint64, start, end string, desc bool, iter := func(item btree.Item) bool { if i >= cursor { iitm := item.(*itemT) - active = iterator(iitm.id, iitm.object, iitm.fields) + active = iterator(iitm.id, iitm.object, c.getFieldValues(iitm.id)) } i++ return active @@ -316,7 +317,7 @@ func (c *Collection) SearchValues(cursor uint64, desc bool, iter := func(item btree.Item) bool { if i >= cursor { iitm := item.(*itemT) - active = iterator(iitm.id, iitm.object, iitm.fields) + active = iterator(iitm.id, iitm.object, c.getFieldValues(iitm.id)) } i++ return active @@ -338,7 +339,7 @@ func (c *Collection) SearchValuesRange(cursor uint64, start, end string, desc bo iter := func(item btree.Item) bool { if i >= cursor { iitm := item.(*itemT) - active = iterator(iitm.id, iitm.object, iitm.fields) + active = iterator(iitm.id, iitm.object, c.getFieldValues(iitm.id)) } i++ return active @@ -360,7 +361,7 @@ func (c *Collection) ScanGreaterOrEqual(id string, cursor uint64, desc bool, iter := func(item btree.Item) bool { if i >= cursor { iitm := item.(*itemT) - active = iterator(iitm.id, iitm.object, iitm.fields) + active = iterator(iitm.id, iitm.object, c.getFieldValues(iitm.id)) } i++ return active @@ -380,7 +381,7 @@ func (c *Collection) geoSearch(cursor uint64, bbox geojson.BBox, iterator func(i if !ok { return true // just ignore } - if !iterator(iitm.id, iitm.object, iitm.fields) { + if !iterator(iitm.id, iitm.object, c.getFieldValues(iitm.id)) { return false } return true diff --git a/controller/collection/collection_test.go b/controller/collection/collection_test.go index 63743e87..7b5df58c 100644 --- a/controller/collection/collection_test.go +++ b/controller/collection/collection_test.go @@ -90,20 +90,101 @@ func TestManyCollections(t *testing.T) { }) } +type testPointItem struct { + id string + object geojson.Object +} + func BenchmarkInsert(t *testing.B) { rand.Seed(time.Now().UnixNano()) - ids := make([]string, t.N) - points := make([]geojson.Object, t.N) + items := make([]testPointItem, t.N) for i := 0; i < t.N; i++ { - points[i] = geojson.SimplePoint{ - Y: rand.Float64()*180 - 90, - X: rand.Float64()*360 - 180, + items[i] = testPointItem{ + fmt.Sprintf("%d", i), + geojson.SimplePoint{ + Y: rand.Float64()*180 - 90, + X: rand.Float64()*360 - 180, + }, } - ids[i] = fmt.Sprintf("%d", i) } col := New() t.ResetTimer() for i := 0; i < t.N; i++ { - col.ReplaceOrInsert(ids[i], points[i], nil, nil) + col.ReplaceOrInsert(items[i].id, items[i].object, nil, nil) + } +} + +func BenchmarkReplace(t *testing.B) { + rand.Seed(time.Now().UnixNano()) + items := make([]testPointItem, t.N) + for i := 0; i < t.N; i++ { + items[i] = testPointItem{ + fmt.Sprintf("%d", i), + geojson.SimplePoint{ + Y: rand.Float64()*180 - 90, + X: rand.Float64()*360 - 180, + }, + } + } + col := New() + for i := 0; i < t.N; i++ { + col.ReplaceOrInsert(items[i].id, items[i].object, nil, nil) + } + t.ResetTimer() + for _, i := range rand.Perm(t.N) { + o, _, _ := col.ReplaceOrInsert(items[i].id, items[i].object, nil, nil) + if o != items[i].object { + t.Fatal("shoot!") + } + } +} + +func BenchmarkGet(t *testing.B) { + rand.Seed(time.Now().UnixNano()) + items := make([]testPointItem, t.N) + for i := 0; i < t.N; i++ { + items[i] = testPointItem{ + fmt.Sprintf("%d", i), + geojson.SimplePoint{ + Y: rand.Float64()*180 - 90, + X: rand.Float64()*360 - 180, + }, + } + } + col := New() + for i := 0; i < t.N; i++ { + col.ReplaceOrInsert(items[i].id, items[i].object, nil, nil) + } + t.ResetTimer() + for _, i := range rand.Perm(t.N) { + o, _, _ := col.Get(items[i].id) + if o != items[i].object { + t.Fatal("shoot!") + } + } +} + +func BenchmarkRemove(t *testing.B) { + rand.Seed(time.Now().UnixNano()) + items := make([]testPointItem, t.N) + for i := 0; i < t.N; i++ { + items[i] = testPointItem{ + fmt.Sprintf("%d", i), + geojson.SimplePoint{ + Y: rand.Float64()*180 - 90, + X: rand.Float64()*360 - 180, + }, + } + } + col := New() + for i := 0; i < t.N; i++ { + col.ReplaceOrInsert(items[i].id, items[i].object, nil, nil) + } + t.ResetTimer() + for _, i := range rand.Perm(t.N) { + o, _, _ := col.Remove(items[i].id) + if o != items[i].object { + t.Fatal("shoot!") + } } } From 738f7eb43cb61b1a32e84fd50d2a340a98d34150 Mon Sep 17 00:00:00 2001 From: Josh Baker Date: Sat, 31 Dec 2016 09:39:39 -0700 Subject: [PATCH 4/4] removed panic --- controller/collection/collection.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controller/collection/collection.go b/controller/collection/collection.go index dca07d9b..1dfc64ff 100644 --- a/controller/collection/collection.go +++ b/controller/collection/collection.go @@ -172,9 +172,9 @@ func (c *Collection) ReplaceOrInsert(id string, obj geojson.Object, fields []str c.weight += len(newFields) * 8 } } else { - if len(fields) == 0 { - panic("if fields is empty, make it nil") - } + //if len(fields) == 0 { + // panic("if fields is empty, make it nil") + //} // map field name to value for i, field := range fields { c.setField(newItem, field, values[i])