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

@@ -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 {

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

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

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 {

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")
 }

js/src/blob.js

@@ -137,8 +137,8 @@ export class BlobLeafSequence extends IndexedSequence<number> {
     super(vr, blobType, items);
   }
 
-  getOffset(idx: number): number {
-    return idx;
+  cumulativeNumberOfLeaves(idx: number): number {
+    return idx + 1;
   }
 
   getCompareFn(other: IndexedSequence): EqualsFn {

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 =>

js/src/indexed-sequence.js

@@ -13,7 +13,7 @@ import {equals} from './compare.js';
 import {notNull} from './assert.js';
 
 export class IndexedSequence<T> extends Sequence<T> {
-  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<T> extends Sequence<T> {
 
 export class IndexedSequenceCursor<T> extends SequenceCursor<T, IndexedSequence> {
   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<T> {

js/src/list.js

@@ -129,8 +129,8 @@ export class ListLeafSequence<T: Value> extends IndexedSequence<T> {
     return getValueChunks(this.items);
   }
 
-  getOffset(idx: number): number {
-    return idx;
+  cumulativeNumberOfLeaves(idx: number): number {
+    return idx + 1;
   }
 
   range(start: number, end: number): Promise<Array<T>> {

js/src/meta-sequence.js

@@ -171,7 +171,7 @@ export class IndexedMetaSequence extends IndexedSequence<MetaTuple> {
 
     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<MetaTuple> {
     });
   }
 
-  getOffset(idx: number): number {
-    return this._offsets[idx] - 1;
+  cumulativeNumberOfLeaves(idx: number): number {
+    return this._offsets[idx];
   }
 
   getCompareFn(other: IndexedSequence): EqualsFn {