From 01b7278c3a6a7b667957fded5b595f4a49fe8a6a Mon Sep 17 00:00:00 2001 From: kortschak Date: Wed, 30 Jul 2014 11:44:36 +0930 Subject: [PATCH] Move UID handling from Base Also clean up some of the value creation code. --- graph/iterator/all_iterator.go | 15 +++++-- graph/iterator/and_iterator.go | 12 ++++-- graph/iterator/fixed_iterator.go | 14 ++++-- graph/iterator/hasa_iterator.go | 15 +++++-- graph/iterator/iterator.go | 17 +++----- graph/iterator/linksto_iterator.go | 17 +++++--- graph/iterator/optional_iterator.go | 6 +++ graph/iterator/or_iterator.go | 24 +++++++---- graph/iterator/value_comparison_iterator.go | 17 +++++--- graph/leveldb/all_iterator.go | 30 +++++++++---- graph/leveldb/iterator.go | 41 ++++++++++++------ graph/memstore/iterator.go | 15 +++++-- graph/mongo/iterator.go | 67 ++++++++++++++++++----------- 13 files changed, 198 insertions(+), 92 deletions(-) diff --git a/graph/iterator/all_iterator.go b/graph/iterator/all_iterator.go index 0548fa5..9bf6441 100644 --- a/graph/iterator/all_iterator.go +++ b/graph/iterator/all_iterator.go @@ -32,6 +32,7 @@ import ( // An All iterator across a range of int64 values, from `max` to `min`. type Int64 struct { Base + uid uint64 tags graph.Tagger max, min int64 at int64 @@ -39,14 +40,20 @@ type Int64 struct { // Creates a new Int64 with the given range. func NewInt64(min, max int64) *Int64 { - var all Int64 + all := Int64{ + uid: NextUID(), + min: min, + max: max, + at: min, + } BaseInit(&all.Base) - all.max = max - all.min = min - all.at = min return &all } +func (it *Int64) UID() uint64 { + return it.uid +} + // Start back at the beginning func (it *Int64) Reset() { it.at = it.min diff --git a/graph/iterator/and_iterator.go b/graph/iterator/and_iterator.go index 3f129cc..a7dce74 100644 --- a/graph/iterator/and_iterator.go +++ b/graph/iterator/and_iterator.go @@ -26,6 +26,7 @@ import ( // be Next()ed if next is called. type And struct { Base + uid uint64 tags graph.Tagger internalIterators []graph.Iterator itCount int @@ -35,13 +36,18 @@ type And struct { // Creates a new And iterator. func NewAnd() *And { - var and And + and := And{ + uid: NextUID(), + internalIterators: make([]graph.Iterator, 0, 20), + } BaseInit(&and.Base) - and.internalIterators = make([]graph.Iterator, 0, 20) - and.checkList = nil return &and } +func (it *And) UID() uint64 { + return it.uid +} + // Reset all internal iterators func (it *And) Reset() { it.primaryIt.Reset() diff --git a/graph/iterator/fixed_iterator.go b/graph/iterator/fixed_iterator.go index 760962f..61e9b50 100644 --- a/graph/iterator/fixed_iterator.go +++ b/graph/iterator/fixed_iterator.go @@ -31,6 +31,7 @@ import ( // an equality function. type Fixed struct { Base + uid uint64 tags graph.Tagger values []graph.Value lastIndex int @@ -55,14 +56,19 @@ func newFixed() *Fixed { // Creates a new Fixed iterator with a custom comparitor. func NewFixedIteratorWithCompare(compareFn Equality) *Fixed { - var it Fixed + it := Fixed{ + uid: NextUID(), + values: make([]graph.Value, 0, 20), + cmp: compareFn, + } BaseInit(&it.Base) - it.values = make([]graph.Value, 0, 20) - it.lastIndex = 0 - it.cmp = compareFn return &it } +func (it *Fixed) UID() uint64 { + return it.uid +} + func (it *Fixed) Reset() { it.lastIndex = 0 } diff --git a/graph/iterator/hasa_iterator.go b/graph/iterator/hasa_iterator.go index 07883e3..f883fd7 100644 --- a/graph/iterator/hasa_iterator.go +++ b/graph/iterator/hasa_iterator.go @@ -47,6 +47,7 @@ import ( // and a temporary holder for the iterator generated on Check(). type HasA struct { Base + uid uint64 tags graph.Tagger ts graph.TripleStore primaryIt graph.Iterator @@ -57,14 +58,20 @@ type HasA struct { // Construct a new HasA iterator, given the triple subiterator, and the triple // direction for which it stands. func NewHasA(ts graph.TripleStore, subIt graph.Iterator, d graph.Direction) *HasA { - var hasa HasA + hasa := HasA{ + uid: NextUID(), + ts: ts, + primaryIt: subIt, + dir: d, + } BaseInit(&hasa.Base) - hasa.ts = ts - hasa.primaryIt = subIt - hasa.dir = d return &hasa } +func (it *HasA) UID() uint64 { + return it.uid +} + // Return our sole subiterator. func (it *HasA) SubIterators() []graph.Iterator { return []graph.Iterator{it.primaryIt} diff --git a/graph/iterator/iterator.go b/graph/iterator/iterator.go index 24a7a7f..130dd6c 100644 --- a/graph/iterator/iterator.go +++ b/graph/iterator/iterator.go @@ -22,8 +22,6 @@ import ( "strings" "sync/atomic" - "github.com/barakmich/glog" - "github.com/google/cayley/graph" ) @@ -38,20 +36,12 @@ func NextUID() uint64 { type Base struct { Last graph.Value canNext bool - uid uint64 } // Called by subclases. func BaseInit(it *Base) { // Your basic iterator is nextable it.canNext = true - if glog.V(2) { - it.uid = NextUID() - } -} - -func (it *Base) UID() uint64 { - return it.uid } // Prints a silly debug string. Most classes override. @@ -115,12 +105,17 @@ func (it *Base) Reset() {} // so it's important to give it a special iterator. type Null struct { Base + uid uint64 tags graph.Tagger } // Fairly useless New function. func NewNull() *Null { - return &Null{} + return &Null{uid: NextUID()} +} + +func (it *Null) UID() uint64 { + return it.uid } func (it *Null) Tagger() *graph.Tagger { diff --git a/graph/iterator/linksto_iterator.go b/graph/iterator/linksto_iterator.go index 63757ae..e61dc09 100644 --- a/graph/iterator/linksto_iterator.go +++ b/graph/iterator/linksto_iterator.go @@ -41,6 +41,7 @@ import ( // `next_it` is the tempoarary iterator held per result in `primary_it`. type LinksTo struct { Base + uid uint64 tags graph.Tagger ts graph.TripleStore primaryIt graph.Iterator @@ -51,15 +52,21 @@ type LinksTo struct { // Construct a new LinksTo iterator around a direction and a subiterator of // nodes. func NewLinksTo(ts graph.TripleStore, it graph.Iterator, d graph.Direction) *LinksTo { - var lto LinksTo + lto := LinksTo{ + uid: NextUID(), + ts: ts, + primaryIt: it, + dir: d, + nextIt: &Null{}, + } BaseInit(<o.Base) - lto.ts = ts - lto.primaryIt = it - lto.dir = d - lto.nextIt = &Null{} return <o } +func (it *LinksTo) UID() uint64 { + return it.uid +} + func (it *LinksTo) Reset() { it.primaryIt.Reset() if it.nextIt != nil { diff --git a/graph/iterator/optional_iterator.go b/graph/iterator/optional_iterator.go index fc3c41b..defee8c 100644 --- a/graph/iterator/optional_iterator.go +++ b/graph/iterator/optional_iterator.go @@ -39,6 +39,7 @@ import ( // and whether the last check we received was true or false. type Optional struct { Base + uid uint64 tags graph.Tagger subIt graph.Iterator lastCheck bool @@ -50,9 +51,14 @@ func NewOptional(it graph.Iterator) *Optional { BaseInit(&o.Base) o.canNext = false o.subIt = it + o.uid = NextUID() return &o } +func (it *Optional) UID() uint64 { + return it.uid +} + func (it *Optional) Reset() { it.subIt.Reset() it.lastCheck = false diff --git a/graph/iterator/or_iterator.go b/graph/iterator/or_iterator.go index da1da5c..e84c629 100644 --- a/graph/iterator/or_iterator.go +++ b/graph/iterator/or_iterator.go @@ -30,6 +30,7 @@ import ( type Or struct { Base + uid uint64 tags graph.Tagger isShortCircuiting bool internalIterators []graph.Iterator @@ -38,23 +39,30 @@ type Or struct { } func NewOr() *Or { - var or Or + or := Or{ + uid: NextUID(), + internalIterators: make([]graph.Iterator, 0, 20), + currentIterator: -1, + } BaseInit(&or.Base) - or.internalIterators = make([]graph.Iterator, 0, 20) - or.isShortCircuiting = false - or.currentIterator = -1 return &or } func NewShortCircuitOr() *Or { - var or Or + or := Or{ + uid: NextUID(), + internalIterators: make([]graph.Iterator, 0, 20), + isShortCircuiting: true, + currentIterator: -1, + } BaseInit(&or.Base) - or.internalIterators = make([]graph.Iterator, 0, 20) - or.isShortCircuiting = true - or.currentIterator = -1 return &or } +func (it *Or) UID() uint64 { + return it.uid +} + // Reset all internal iterators func (it *Or) Reset() { for _, sub := range it.internalIterators { diff --git a/graph/iterator/value_comparison_iterator.go b/graph/iterator/value_comparison_iterator.go index 6b89cd7..c29fda0 100644 --- a/graph/iterator/value_comparison_iterator.go +++ b/graph/iterator/value_comparison_iterator.go @@ -47,6 +47,7 @@ const ( type Comparison struct { Base + uid uint64 tags graph.Tagger subIt graph.Iterator op Operator @@ -55,15 +56,21 @@ type Comparison struct { } func NewComparison(sub graph.Iterator, op Operator, val interface{}, ts graph.TripleStore) *Comparison { - var vc Comparison + vc := Comparison{ + uid: NextUID(), + subIt: sub, + op: op, + val: val, + ts: ts, + } BaseInit(&vc.Base) - vc.subIt = sub - vc.op = op - vc.val = val - vc.ts = ts return &vc } +func (it *Comparison) UID() uint64 { + return it.uid +} + // Here's the non-boilerplate part of the ValueComparison iterator. Given a value // and our operator, determine whether or not we meet the requirement. func (it *Comparison) doComparison(val graph.Value) bool { diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index 177eec0..84d815c 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -28,6 +28,7 @@ import ( type AllIterator struct { iterator.Base + uid uint64 tags graph.Tagger prefix []byte dir graph.Direction @@ -38,23 +39,36 @@ type AllIterator struct { } func NewAllIterator(prefix string, d graph.Direction, ts *TripleStore) *AllIterator { - var it AllIterator + opts := &opt.ReadOptions{ + DontFillCache: true, + } + + it := AllIterator{ + uid: iterator.NextUID(), + ro: opts, + iter: ts.db.NewIterator(nil, opts), + prefix: []byte(prefix), + dir: d, + open: true, + ts: ts, + } iterator.BaseInit(&it.Base) - it.ro = &opt.ReadOptions{} - it.ro.DontFillCache = true - it.iter = ts.db.NewIterator(nil, it.ro) - it.prefix = []byte(prefix) - it.dir = d - it.open = true - it.ts = ts + it.iter.Seek(it.prefix) if !it.iter.Valid() { + // FIXME(kortschak) What are the semantics here? Is this iterator usable? + // If not, we should return nil *Iterator and an error. it.open = false it.iter.Release() } + return &it } +func (it *AllIterator) UID() uint64 { + return it.uid +} + func (it *AllIterator) Reset() { if !it.open { it.iter = it.ts.db.NewIterator(nil, it.ro) diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index 53301cc..c4c98a3 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -28,6 +28,7 @@ import ( type Iterator struct { iterator.Base + uid uint64 tags graph.Tagger nextPrefix []byte checkId []byte @@ -40,27 +41,43 @@ type Iterator struct { } func NewIterator(prefix string, d graph.Direction, value graph.Value, ts *TripleStore) *Iterator { - var it Iterator + vb := value.([]byte) + p := make([]byte, 0, 2+ts.hasher.Size()) + p = append(p, []byte(prefix)...) + p = append(p, []byte(vb[1:])...) + + opts := &opt.ReadOptions{ + DontFillCache: true, + } + + it := Iterator{ + uid: iterator.NextUID(), + nextPrefix: p, + checkId: vb, + dir: d, + originalPrefix: prefix, + ro: opts, + iter: ts.db.NewIterator(nil, opts), + open: true, + ts: ts, + } iterator.BaseInit(&it.Base) - it.checkId = value.([]byte) - it.dir = d - it.originalPrefix = prefix - it.nextPrefix = make([]byte, 0, 2+ts.hasher.Size()) - it.nextPrefix = append(it.nextPrefix, []byte(prefix)...) - it.nextPrefix = append(it.nextPrefix, []byte(it.checkId[1:])...) - it.ro = &opt.ReadOptions{} - it.ro.DontFillCache = true - it.iter = ts.db.NewIterator(nil, it.ro) - it.open = true - it.ts = ts + ok := it.iter.Seek(it.nextPrefix) if !ok { + // FIXME(kortschak) What are the semantics here? Is this iterator usable? + // If not, we should return nil *Iterator and an error. it.open = false it.iter.Release() } + return &it } +func (it *Iterator) UID() uint64 { + return it.uid +} + func (it *Iterator) Reset() { if !it.open { it.iter = it.ts.db.NewIterator(nil, it.ro) diff --git a/graph/memstore/iterator.go b/graph/memstore/iterator.go index a5db108..7d08a09 100644 --- a/graph/memstore/iterator.go +++ b/graph/memstore/iterator.go @@ -27,6 +27,7 @@ import ( type Iterator struct { iterator.Base + uid uint64 tags graph.Tagger tree *llrb.LLRB data string @@ -54,14 +55,20 @@ func IterateOne(tree *llrb.LLRB, last Int64) Int64 { } func NewLlrbIterator(tree *llrb.LLRB, data string) *Iterator { - var it Iterator + it := Iterator{ + uid: iterator.NextUID(), + tree: tree, + iterLast: Int64(-1), + data: data, + } iterator.BaseInit(&it.Base) - it.tree = tree - it.iterLast = Int64(-1) - it.data = data return &it } +func (it *Iterator) UID() uint64 { + return it.uid +} + func (it *Iterator) Reset() { it.iterLast = Int64(-1) } diff --git a/graph/mongo/iterator.go b/graph/mongo/iterator.go index 4996572..a5b6dc7 100644 --- a/graph/mongo/iterator.go +++ b/graph/mongo/iterator.go @@ -28,6 +28,7 @@ import ( type Iterator struct { iterator.Base + uid uint64 tags graph.Tagger ts *TripleStore dir graph.Direction @@ -41,54 +42,72 @@ type Iterator struct { } func NewIterator(ts *TripleStore, collection string, d graph.Direction, val graph.Value) *Iterator { - var m Iterator - iterator.BaseInit(&m.Base) + name := ts.NameOf(val) - m.name = ts.NameOf(val) - m.collection = collection + var constraint bson.M switch d { case graph.Subject: - m.constraint = bson.M{"Subject": m.name} + constraint = bson.M{"Subject": name} case graph.Predicate: - m.constraint = bson.M{"Predicate": m.name} + constraint = bson.M{"Predicate": name} case graph.Object: - m.constraint = bson.M{"Object": m.name} + constraint = bson.M{"Object": name} case graph.Provenance: - m.constraint = bson.M{"Provenance": m.name} + constraint = bson.M{"Provenance": name} } - m.ts = ts - m.dir = d - m.iter = ts.db.C(collection).Find(m.constraint).Iter() - size, err := ts.db.C(collection).Find(m.constraint).Count() + size, err := ts.db.C(collection).Find(constraint).Count() if err != nil { + // FIXME(kortschak) This should be passed back rather than just logging. glog.Errorln("Trouble getting size for iterator! ", err) return nil } - m.size = int64(size) - m.hash = val.(string) - m.isAll = false + + m := Iterator{ + uid: iterator.NextUID(), + name: name, + constraint: constraint, + collection: collection, + ts: ts, + dir: d, + iter: ts.db.C(collection).Find(constraint).Iter(), + size: int64(size), + hash: val.(string), + isAll: false, + } + iterator.BaseInit(&m.Base) + return &m } func NewAllIterator(ts *TripleStore, collection string) *Iterator { - var m Iterator - m.ts = ts - m.dir = graph.Any - m.constraint = nil - m.collection = collection - m.iter = ts.db.C(collection).Find(nil).Iter() size, err := ts.db.C(collection).Count() if err != nil { + // FIXME(kortschak) This should be passed back rather than just logging. glog.Errorln("Trouble getting size for iterator! ", err) return nil } - m.size = int64(size) - m.hash = "" - m.isAll = true + + m := Iterator{ + uid: iterator.NextUID(), + ts: ts, + dir: graph.Any, + constraint: nil, + collection: collection, + iter: ts.db.C(collection).Find(nil).Iter(), + size: int64(size), + hash: "", + isAll: true, + } + // FIXME(kortschak) Was there supposed to be a BaseInit call here? + return &m } +func (it *Iterator) UID() uint64 { + return it.uid +} + func (it *Iterator) Reset() { it.iter.Close() it.iter = it.ts.db.C(it.collection).Find(it.constraint).Iter()