From 74ba1012c2b5d43a435e770d33d8fd5e6a51c352 Mon Sep 17 00:00:00 2001 From: Rafael Weinstein Date: Thu, 28 Jul 2016 16:55:41 -0700 Subject: [PATCH] sequence.getOffset => sequence.cumulativeNumLeaves (#2193) sequence.getOffset was problematic and didn't have a clear meaning. In addition it was causing a bunch of +1 code at call sites. This patch replaces it with cumulativeNumLeaves which has a clearer meaning. --- go/types/blob_leaf_sequence.go | 4 ++-- go/types/indexed_sequence_diff.go | 4 ++-- go/types/indexed_sequences.go | 18 +++++------------- go/types/list_leaf_sequence.go | 4 ++-- go/types/meta_sequence.go | 2 +- js/src/blob.js | 4 ++-- js/src/indexed-sequence-diff.js | 4 ++-- js/src/indexed-sequence.js | 6 +++--- js/src/list.js | 4 ++-- js/src/meta-sequence.js | 6 +++--- 10 files changed, 24 insertions(+), 32 deletions(-) diff --git a/go/types/blob_leaf_sequence.go b/go/types/blob_leaf_sequence.go index afb4e63649..15bbbe4012 100644 --- a/go/types/blob_leaf_sequence.go +++ b/go/types/blob_leaf_sequence.go @@ -14,8 +14,8 @@ func newBlobLeafSequence(vr ValueReader, data []byte) indexedSequence { } // indexedSequence interface -func (bl blobLeafSequence) getOffset(idx int) uint64 { - return uint64(idx) +func (bl blobLeafSequence) cumulativeNumberOfLeaves(idx int) uint64 { + return uint64(idx) + 1 } func (bl blobLeafSequence) getCompareFn(other sequence) compareFn { diff --git a/go/types/indexed_sequence_diff.go b/go/types/indexed_sequence_diff.go index 99c61bbe9b..d00d9c56ff 100644 --- a/go/types/indexed_sequence_diff.go +++ b/go/types/indexed_sequence_diff.go @@ -37,11 +37,11 @@ func indexedSequenceDiff(last indexedSequence, lastHeight int, lastOffset uint64 currentChild := current.(indexedMetaSequence).getCompositeChildSequence(splice.SpFrom, splice.SpAdded).(indexedSequence) lastChildOffset := lastOffset if splice.SpAt > 0 { - lastChildOffset += last.getOffset(int(splice.SpAt)-1) + 1 + lastChildOffset += last.cumulativeNumberOfLeaves(int(splice.SpAt) - 1) } currentChildOffset := currentOffset if splice.SpFrom > 0 { - currentChildOffset += current.getOffset(int(splice.SpFrom)-1) + 1 + currentChildOffset += current.cumulativeNumberOfLeaves(int(splice.SpFrom) - 1) } if ok := indexedSequenceDiff(lastChild, lastHeight-1, lastChildOffset, currentChild, currentHeight-1, currentChildOffset, changes, closeChan, maxSpliceMatrixSize); !ok { return false diff --git a/go/types/indexed_sequences.go b/go/types/indexed_sequences.go index cdc07f7320..7e2963a157 100644 --- a/go/types/indexed_sequences.go +++ b/go/types/indexed_sequences.go @@ -12,7 +12,7 @@ import ( type indexedSequence interface { sequence - getOffset(idx int) uint64 + cumulativeNumberOfLeaves(idx int) uint64 // returns the total number of leaf values reachable from this sequence for all sub-trees from 0 to |idx| } type indexedMetaSequence struct { @@ -48,16 +48,8 @@ func newIndexedMetaSequence(tuples metaSequenceData, t *Type, vr ValueReader) in } } -func (ims indexedMetaSequence) getOffset(idx int) uint64 { - // TODO: precompute these on the construction - offsets := []uint64{} - cum := uint64(0) - for _, mt := range ims.tuples { - cum += mt.key.uint64Value() - offsets = append(offsets, cum) - } - - return ims.offsets[idx] - 1 +func (ims indexedMetaSequence) cumulativeNumberOfLeaves(idx int) uint64 { + return ims.offsets[idx] } func (ims indexedMetaSequence) getCompareFn(other sequence) compareFn { @@ -86,7 +78,7 @@ func newCursorAtIndex(seq indexedSequence, idx uint64) *sequenceCursor { func advanceCursorToOffset(cur *sequenceCursor, idx uint64) uint64 { seq := cur.seq.(indexedSequence) cur.idx = sort.Search(seq.seqLen(), func(i int) bool { - return uint64(idx) <= seq.getOffset(i) + return uint64(idx) < seq.cumulativeNumberOfLeaves(i) }) if _, ok := seq.(metaSequence); ok { if cur.idx == seq.seqLen() { @@ -96,7 +88,7 @@ func advanceCursorToOffset(cur *sequenceCursor, idx uint64) uint64 { if cur.idx == 0 { return 0 } - return seq.getOffset(cur.idx-1) + 1 + return seq.cumulativeNumberOfLeaves(cur.idx - 1) } // If |sink| is not nil, chunks will be eagerly written as they're created. Otherwise they are diff --git a/go/types/list_leaf_sequence.go b/go/types/list_leaf_sequence.go index cbbec9b98f..fa4d070364 100644 --- a/go/types/list_leaf_sequence.go +++ b/go/types/list_leaf_sequence.go @@ -20,8 +20,8 @@ func newListLeafSequence(vr ValueReader, v ...Value) indexedSequence { } // indexedSequence interface -func (ll listLeafSequence) getOffset(idx int) uint64 { - return uint64(idx) +func (ll listLeafSequence) cumulativeNumberOfLeaves(idx int) uint64 { + return uint64(idx) + 1 } func (ll listLeafSequence) getCompareFn(other sequence) compareFn { diff --git a/go/types/meta_sequence.go b/go/types/meta_sequence.go index 3c133f1619..40336d4c17 100644 --- a/go/types/meta_sequence.go +++ b/go/types/meta_sequence.go @@ -280,6 +280,6 @@ func (es emptySequence) getKey(idx int) orderedKey { panic("empty sequence") } -func (es emptySequence) getOffset(idx int) uint64 { +func (es emptySequence) cumulativeNumberOfLeaves(idx int) uint64 { panic("empty sequence") } diff --git a/js/src/blob.js b/js/src/blob.js index f8f035b06f..dc226dd23a 100644 --- a/js/src/blob.js +++ b/js/src/blob.js @@ -137,8 +137,8 @@ export class BlobLeafSequence extends IndexedSequence { super(vr, blobType, items); } - getOffset(idx: number): number { - return idx; + cumulativeNumberOfLeaves(idx: number): number { + return idx + 1; } getCompareFn(other: IndexedSequence): EqualsFn { diff --git a/js/src/indexed-sequence-diff.js b/js/src/indexed-sequence-diff.js index fd94b87a27..4ea96005ae 100644 --- a/js/src/indexed-sequence-diff.js +++ b/js/src/indexed-sequence-diff.js @@ -54,11 +54,11 @@ export function diff(last: IndexedSequence, lastHeight: number, lastOffset: numb let lastChildOffset = lastOffset; if (splice[SPLICE_AT] > 0) { - lastChildOffset += last.getOffset(splice[SPLICE_AT] - 1) + 1; + lastChildOffset += last.cumulativeNumberOfLeaves(splice[SPLICE_AT] - 1); } let currentChildOffset = currentOffset; if (splice[SPLICE_FROM] > 0) { - currentChildOffset += current.getOffset(splice[SPLICE_FROM] - 1) + 1; + currentChildOffset += current.cumulativeNumberOfLeaves(splice[SPLICE_FROM] - 1); } return Promise.all([lastChildP, currentChildP]).then(childSequences => diff --git a/js/src/indexed-sequence.js b/js/src/indexed-sequence.js index 8c1b0efab9..5668975326 100644 --- a/js/src/indexed-sequence.js +++ b/js/src/indexed-sequence.js @@ -13,7 +13,7 @@ import {equals} from './compare.js'; import {notNull} from './assert.js'; export class IndexedSequence extends Sequence { - getOffset(idx: number): number { // eslint-disable-line no-unused-vars + cumulativeNumberOfLeaves(idx: number): number { // eslint-disable-line no-unused-vars throw new Error('override'); } @@ -43,13 +43,13 @@ export class IndexedSequence extends Sequence { export class IndexedSequenceCursor extends SequenceCursor { advanceToOffset(idx: number): number { - this.idx = search(this.length, (i: number) => idx <= this.sequence.getOffset(i)); + this.idx = search(this.length, (i: number) => idx < this.sequence.cumulativeNumberOfLeaves(i)); if (this.sequence.isMeta && this.idx === this.length) { this.idx = this.length - 1; } - return this.idx > 0 ? this.sequence.getOffset(this.idx - 1) + 1 : 0; + return this.idx > 0 ? this.sequence.cumulativeNumberOfLeaves(this.idx - 1) : 0; } clone(): IndexedSequenceCursor { diff --git a/js/src/list.js b/js/src/list.js index 90dd1a453a..6d03532da8 100644 --- a/js/src/list.js +++ b/js/src/list.js @@ -129,8 +129,8 @@ export class ListLeafSequence extends IndexedSequence { return getValueChunks(this.items); } - getOffset(idx: number): number { - return idx; + cumulativeNumberOfLeaves(idx: number): number { + return idx + 1; } range(start: number, end: number): Promise> { diff --git a/js/src/meta-sequence.js b/js/src/meta-sequence.js index 30696f8d1f..fc95badbd6 100644 --- a/js/src/meta-sequence.js +++ b/js/src/meta-sequence.js @@ -171,7 +171,7 @@ export class IndexedMetaSequence extends IndexedSequence { const childRanges = []; for (let i = 0; i < this.items.length && end > start; i++) { - const cum = this.getOffset(i) + 1; + const cum = this.cumulativeNumberOfLeaves(i); const seqLength = this.items[i].key.numberValue(); if (start < cum) { const seqStart = cum - seqLength; @@ -230,8 +230,8 @@ export class IndexedMetaSequence extends IndexedSequence { }); } - getOffset(idx: number): number { - return this._offsets[idx] - 1; + cumulativeNumberOfLeaves(idx: number): number { + return this._offsets[idx]; } getCompareFn(other: IndexedSequence): EqualsFn {