From 08d7520073a2f4b78c24c8309e0d40fd97c7408a Mon Sep 17 00:00:00 2001 From: Andy Arthur Date: Thu, 11 May 2023 15:54:21 -0700 Subject: [PATCH] go/store/prolly: cleanup todos, remove error from cursor.isLeaf --- go/store/prolly/map_test.go | 1 - go/store/prolly/message/merge_artifacts.go | 1 - go/store/prolly/message/prolly_map.go | 1 - go/store/prolly/message/serialize.go | 1 - go/store/prolly/tree/content_address_test.go | 2 +- go/store/prolly/tree/node_cursor.go | 63 ++++---------------- go/store/prolly/tree/node_splitter.go | 1 - 7 files changed, 11 insertions(+), 59 deletions(-) diff --git a/go/store/prolly/map_test.go b/go/store/prolly/map_test.go index 3fe2fa6ac8..34858418f5 100644 --- a/go/store/prolly/map_test.go +++ b/go/store/prolly/map_test.go @@ -33,7 +33,6 @@ import ( "github.com/dolthub/dolt/go/store/val" ) -// todo(andy): randomize test seed var testRand = rand.New(rand.NewSource(1)) var sharedPool = pool.NewBuffPool() diff --git a/go/store/prolly/message/merge_artifacts.go b/go/store/prolly/message/merge_artifacts.go index 5db43efe0e..e18815e7df 100644 --- a/go/store/prolly/message/merge_artifacts.go +++ b/go/store/prolly/message/merge_artifacts.go @@ -224,7 +224,6 @@ func estimateMergeArtifactSize(keys, values [][]byte, subtrees []uint64, keyAddr panic(fmt.Sprintf("value vector exceeds Size limit ( %d > %d )", valSz, MaxVectorOffset)) } - // todo(andy): better estimates bufSz += keySz + valSz // tuples bufSz += refCntSz // subtree counts bufSz += len(keys)*2 + len(values)*2 // offStart diff --git a/go/store/prolly/message/prolly_map.go b/go/store/prolly/message/prolly_map.go index d631c71d5a..f5e914abe2 100644 --- a/go/store/prolly/message/prolly_map.go +++ b/go/store/prolly/message/prolly_map.go @@ -222,7 +222,6 @@ func estimateProllyMapSize(keys, values [][]byte, subtrees []uint64, valAddrsCnt panic(fmt.Sprintf("value vector exceeds Size limit ( %d > %d )", valSz, MaxVectorOffset)) } - // todo(andy): better estimates bufSz += keySz + valSz // tuples bufSz += subtreesSz // subtree counts bufSz += len(keys)*2 + len(values)*2 // offStart diff --git a/go/store/prolly/message/serialize.go b/go/store/prolly/message/serialize.go index 27903f7cb4..70b8fcaf36 100644 --- a/go/store/prolly/message/serialize.go +++ b/go/store/prolly/message/serialize.go @@ -100,7 +100,6 @@ func writeAddressOffsets(b *fb.Builder, items [][]byte, sumSz int, td val.TupleD } func writeCountArray(b *fb.Builder, counts []uint64) fb.UOffsetT { - // todo(andy): write without alloc buf := make([]byte, maxEncodedSize(len(counts))) return b.CreateByteVector(encodeVarints(counts, buf)) } diff --git a/go/store/prolly/tree/content_address_test.go b/go/store/prolly/tree/content_address_test.go index e378180b69..31b487f621 100644 --- a/go/store/prolly/tree/content_address_test.go +++ b/go/store/prolly/tree/content_address_test.go @@ -33,7 +33,7 @@ var goldenHash = hash.Hash{ 0xea, 0x7d, 0x47, 0x69, 0x6c, } -// todo(andy): need and analogous test in pkg prolly +// todo(andy): need an analogous test in pkg prolly func TestContentAddress(t *testing.T) { tups, _ := AscendingUintTuples(12345) m := makeTree(t, tups) diff --git a/go/store/prolly/tree/node_cursor.go b/go/store/prolly/tree/node_cursor.go index 10bf882e46..672212e6fd 100644 --- a/go/store/prolly/tree/node_cursor.go +++ b/go/store/prolly/tree/node_cursor.go @@ -45,12 +45,7 @@ type Ordering[K ~[]byte] interface { func newCursorAtStart(ctx context.Context, ns NodeStore, nd Node) (cur *Cursor, err error) { cur = &Cursor{nd: nd, nrw: ns} - var leaf bool - leaf, err = cur.isLeaf() - if err != nil { - return nil, err - } - for !leaf { + for !cur.isLeaf() { nd, err = fetchChild(ctx, ns, cur.currentRef()) if err != nil { return nil, err @@ -58,10 +53,6 @@ func newCursorAtStart(ctx context.Context, ns NodeStore, nd Node) (cur *Cursor, parent := cur cur = &Cursor{nd: nd, parent: parent, nrw: ns} - leaf, err = cur.isLeaf() - if err != nil { - return nil, err - } } return } @@ -70,12 +61,7 @@ func newCursorAtEnd(ctx context.Context, ns NodeStore, nd Node) (cur *Cursor, er cur = &Cursor{nd: nd, nrw: ns} cur.skipToNodeEnd() - var leaf bool - leaf, err = cur.isLeaf() - if err != nil { - return nil, err - } - for !leaf { + for !cur.isLeaf() { nd, err = fetchChild(ctx, ns, cur.currentRef()) if err != nil { return nil, err @@ -84,10 +70,6 @@ func newCursorAtEnd(ctx context.Context, ns NodeStore, nd Node) (cur *Cursor, er parent := cur cur = &Cursor{nd: nd, parent: parent, nrw: ns} cur.skipToNodeEnd() - leaf, err = cur.isLeaf() - if err != nil { - return nil, err - } } return } @@ -140,11 +122,7 @@ func newCursorAtOrdinal(ctx context.Context, ns NodeStore, nd Node, ord uint64) // GetOrdinalOfCursor returns the ordinal position of a Cursor. func getOrdinalOfCursor(curr *Cursor) (ord uint64, err error) { - leaf, err := curr.isLeaf() - if err != nil { - return 0, err - } - if !leaf { + if !curr.isLeaf() { return 0, fmt.Errorf("|cur| must be at a leaf") } @@ -186,13 +164,7 @@ func newCursorFromSearchFn(ctx context.Context, ns NodeStore, nd Node, search Se cur = &Cursor{nd: nd, nrw: ns} cur.idx = search(cur.nd) - var leaf bool - leaf, err = cur.isLeaf() - if err != nil { - return nil, err - } - for !leaf { - + for !cur.isLeaf() { // stay in bounds for internal nodes cur.keepInBounds() @@ -205,16 +177,12 @@ func newCursorFromSearchFn(ctx context.Context, ns NodeStore, nd Node, search Se cur = &Cursor{nd: nd, parent: parent, nrw: ns} cur.idx = search(cur.nd) - leaf, err = cur.isLeaf() - if err != nil { - return nil, err - } } - return } func newLeafCursorAtKey[K ~[]byte, O Ordering[K]](ctx context.Context, ns NodeStore, nd Node, key K, order O) (Cursor, error) { + var err error cur := Cursor{nd: nd, nrw: ns} for { // binary search |cur.nd| for |key| @@ -230,10 +198,7 @@ func newLeafCursorAtKey[K ~[]byte, O Ordering[K]](ctx context.Context, ns NodeSt } cur.idx = i - leaf, err := cur.isLeaf() - if err != nil { - return cur, err - } else if leaf { + if cur.isLeaf() { break // done } @@ -402,13 +367,10 @@ func (cur *Cursor) currentRef() hash.Hash { } func (cur *Cursor) currentSubtreeSize() (uint64, error) { - leaf, err := cur.isLeaf() - if err != nil { - return 0, err - } - if leaf { + if cur.isLeaf() { return 1, nil } + var err error cur.nd, err = cur.nd.loadSubtrees() if err != nil { return 0, err @@ -455,13 +417,8 @@ func (cur *Cursor) atNodeEnd() bool { return cur.idx == lastKeyIdx } -func (cur *Cursor) isLeaf() (bool, error) { - // todo(andy): cache Level - lvl, err := cur.level() - if err != nil { - return false, err - } - return lvl == 0, nil +func (cur *Cursor) isLeaf() bool { + return cur.nd.level == 0 } func (cur *Cursor) level() (uint64, error) { diff --git a/go/store/prolly/tree/node_splitter.go b/go/store/prolly/tree/node_splitter.go index 2154f30389..b21b70ad05 100644 --- a/go/store/prolly/tree/node_splitter.go +++ b/go/store/prolly/tree/node_splitter.go @@ -190,7 +190,6 @@ func newKeySplitter(level uint8) nodeSplitter { var _ splitterFactory = newKeySplitter func (ks *keySplitter) Append(key, value Item) error { - // todo(andy): account for key/value offsets, vtable, etc. thisSize := uint32(len(key) + len(value)) ks.size += thisSize