JS: Maintain validation cache on DataStore across Commit

While this causes the cache to potentially grow infinitely, having a
DataStore keep track of every value it's read or written makes it
simpler to program with. Once RefValues can only come to be by being
read out of a DataStore, we can do away with this altogether.

The big changes here are that DataStore now has a ValueStore, instead
if extending ValueStore. The DataStore constructor now takes a
ValueStore, so that new instances re-use the same ValueStore can be
constructed on Commit. Since callers don't care about that detail,
this change exports NewDataStore(), which still takes a BatchStore.

js/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@attic/noms",
-  "version": "21.0.0",
+  "version": "21.1.0",
   "description": "Noms JS SDK",
   "repository": "https://github.com/attic-labs/noms",
   "main": "dist/commonjs/noms.js",

js/src/blob.js

@@ -4,7 +4,7 @@ import {Collection} from './collection.js';
 import {IndexedSequence} from './indexed-sequence.js';
 import {SequenceCursor} from './sequence.js';
 import {invariant} from './assert.js';
-import type {ValueReader} from './decode.js';
+import type {ValueReader} from './value-store.js';
 import {blobType} from './type.js';
 import {
   MetaTuple,

js/src/chunk-store.js

@@ -10,3 +10,8 @@ export type ChunkStore = {
   has(ref: Ref): Promise<boolean>;
   put(c: Chunk): void;
 }
+
+export interface RootTracker {
+  getRoot(): Promise<Ref>;
+  updateRoot(current: Ref, last: Ref): Promise<boolean>;
+}

js/src/data-store.js

@@ -6,6 +6,7 @@ import {newStruct} from './struct.js';
 import type {NomsMap} from './map.js';
 import type {NomsSet} from './set.js';
 import type {valueOrPrimitive} from './value.js';
+import type {RootTracker} from './chunk-store.js';
 import ValueStore from './value-store.js';
 import BatchStore from './batch-store.js';
 import {
@@ -62,19 +63,25 @@ export function getDatasTypes(): DatasTypes {
   return datasTypes;
 }
 
-export default class DataStore extends ValueStore {
-  _bs: BatchStore;
-  _cacheSize: number;
+export default class DataStore {
+  _vs: ValueStore;
+  _rt: RootTracker;
   _datasets: Promise<NomsMap<string, RefValue<Commit>>>;
 
   constructor(bs: BatchStore, cacheSize: number = 0) {
-    super(bs, cacheSize);
-    // bs and cacheSize should only be used when creating a new DataStore instance in commit()
-    this._bs = bs;
-    this._cacheSize = cacheSize;
+    this._vs = new ValueStore(bs, cacheSize);
+    this._rt = bs;
     this._datasets = this._datasetsFromRootRef(bs.getRoot());
   }
 
+  _clone(vs: ValueStore, rt: RootTracker): DataStore {
+    const ds = Object.create(DataStore.prototype);
+    ds._vs = vs;
+    ds._rt = rt;
+    ds._datasets = this._datasetsFromRootRef(rt.getRoot());
+    return ds;
+  }
+
   _datasetsFromRootRef(rootRef: Promise<Ref>): Promise<NomsMap<string, RefValue<Commit>>> {
     return rootRef.then(rootRef => {
       if (rootRef.isEmpty()) {
@@ -97,6 +104,16 @@ export default class DataStore extends ValueStore {
     return this._datasets;
   }
 
+  // TODO: This should return Promise<?valueOrPrimitive>
+  async readValue(ref: Ref): Promise<any> {
+    return this._vs.readValue(ref);
+  }
+
+
+  writeValue<T: valueOrPrimitive>(v: T): RefValue<T> {
+    return this._vs.writeValue(v);
+  }
+
   async _descendsFrom(commit: Commit, currentHeadRef: RefValue<Commit>): Promise<boolean> {
     let ancestors = commit.parents;
     while (!(await ancestors.has(currentHeadRef))) {
@@ -109,7 +126,7 @@ export default class DataStore extends ValueStore {
   }
 
   async commit(datasetId: string, commit: Commit): Promise<DataStore> {
-    const currentRootRefP = this._bs.getRoot();
+    const currentRootRefP = this._rt.getRoot();
     const datasetsP = this._datasetsFromRootRef(currentRootRefP);
     let currentDatasets = await (datasetsP:Promise<NomsMap>);
     const currentRootRef = await currentRootRefP;
@@ -129,12 +146,16 @@ export default class DataStore extends ValueStore {
 
     currentDatasets = await currentDatasets.set(datasetId, commitRef);
     const newRootRef = this.writeValue(currentDatasets).targetRef;
-    if (await this._bs.updateRoot(newRootRef, currentRootRef)) {
-      return new DataStore(this._bs, this._cacheSize);
+    if (await this._rt.updateRoot(newRootRef, currentRootRef)) {
+      return this._clone(this._vs, this._rt);
     }
 
     throw new Error('Optimistic lock failed');
   }
+
+  close() {
+    this._vs.close();
+  }
 }
 
 async function getAncestors(commits: NomsSet<RefValue<Commit>>, store: DataStore):

js/src/dataset.js

@@ -2,8 +2,8 @@
 
 import {newCommit} from './data-store.js';
 import type {valueOrPrimitive} from './value.js';
-import type DataStore from './data-store.js';
 import type {Commit} from './commit.js';
+import type DataStore from './data-store.js';
 import RefValue from './ref-value.js';
 
 export default class Dataset {

js/src/decode.js

@@ -25,6 +25,7 @@ import {NomsMap, MapLeafSequence} from './map.js';
 import {NomsSet, SetLeafSequence} from './set.js';
 import {IndexedMetaSequence} from './meta-sequence.js';
 import type {valueOrPrimitive} from './value.js';
+import type {ValueReader} from './value-store.js';
 
 const typedTag = 't ';
 const blobTag = 'b ';
@@ -322,10 +323,6 @@ export class JsonArrayReader {
   }
 }
 
-export interface ValueReader {
-  readValue(ref: Ref): Promise<any>
-}
-
 export function decodeNomsValue(chunk: Chunk, vr: ValueReader): valueOrPrimitive {
   const tag = new Chunk(new Uint8Array(chunk.data.buffer, 0, 2)).toString();
 

js/src/encode.js

@@ -13,12 +13,13 @@ import {ListLeafSequence, NomsList} from './list.js';
 import {MapLeafSequence, NomsMap} from './map.js';
 import {NomsSet, SetLeafSequence} from './set.js';
 import {Sequence} from './sequence.js';
+import {IndexedSequence} from './indexed-sequence.js';
 import {setEncodeNomsValue} from './get-ref.js';
 import {NomsBlob, BlobLeafSequence} from './blob.js';
 import {describeTypeOfValue} from './encode-human-readable.js';
 import type {primitive} from './primitives.js';
 import type {valueOrPrimitive} from './value.js';
-import {IndexedSequence} from './indexed-sequence.js';
+import type {ValueWriter} from './value-store.js';
 
 const typedTag = 't ';
 
@@ -307,10 +308,6 @@ function encodeTopLevelBlob(sequence: BlobLeafSequence): Chunk {
   return new Chunk(data);
 }
 
-interface ValueWriter {
-    writeValue<T: valueOrPrimitive>(v: T, t: ?Type): RefValue<T>
-}
-
 export function encodeNomsValue(v: valueOrPrimitive, t: Type, vw: ?ValueWriter): Chunk {
   if (t.kind === Kind.Blob) {
     invariant(v instanceof NomsBlob || v instanceof IndexedSequence);

js/src/get-ref.js

@@ -2,14 +2,14 @@
 
 import type Chunk from './chunk.js';
 import type Ref from './ref.js';
-import type DataStore from './data-store.js';
+import type {ValueWriter} from './value-store.js';
 import {notNull} from './assert.js';
 import type {Type} from './type.js';
 import type {valueOrPrimitive} from './value.js';
 import {getTypeOfValue} from './type.js';
 import {Value} from './value.js';
 
-type encodeFn = (v: valueOrPrimitive, t: Type, ds: ?DataStore) => Chunk;
+type encodeFn = (v: valueOrPrimitive, t: Type, vw: ?ValueWriter) => Chunk;
 let encodeNomsValue: ?encodeFn = null;
 
 export function getRefOfValue(v: valueOrPrimitive): Ref {

js/src/http-batch-store-test.js

@@ -21,4 +21,4 @@ suite('HttpBatchStore', () => {
     assert.equal(2, chunks.length);
     assert.equal(2, hints.length);
   });
-});
+});

js/src/list.js

@@ -2,7 +2,7 @@
 
 import BuzHashBoundaryChecker from './buzhash-boundary-checker.js';
 import type {BoundaryChecker, makeChunkFn} from './sequence-chunker.js';
-import type {ValueReader} from './decode.js';
+import type {ValueReader} from './value-store.js';
 import type {Splice} from './edit-distance.js';
 import type {valueOrPrimitive} from './value.js'; // eslint-disable-line no-unused-vars
 import type {AsyncIterator} from './async-iterator.js';

js/src/map-test.js

@@ -25,6 +25,7 @@ import {MapLeafSequence, newMap, NomsMap} from './map.js';
 import {MetaTuple, OrderedMetaSequence} from './meta-sequence.js';
 import Ref from './ref.js';
 import type {Type} from './type.js';
+import type {ValueReadWriter} from './value-store.js';
 
 const testMapSize = 1000;
 const mapOfNRef = 'sha1-67b979260f367f9a7e6ac8121eca87a2f5abd015';
@@ -307,30 +308,30 @@ suite('MapLeaf', () => {
 });
 
 suite('CompoundMap', () => {
-  function build(ds: DataStore): Array<NomsMap> {
+  function build(vwr: ValueReadWriter): Array<NomsMap> {
     const tr = makeMapType(stringType,
         boolType);
-    const l1 = new NomsMap(tr, new MapLeafSequence(ds, tr, [{key: 'a', value: false},
+    const l1 = new NomsMap(tr, new MapLeafSequence(vwr, tr, [{key: 'a', value: false},
         {key:'b', value:false}]));
-    const r1 = ds.writeValue(l1);
-    const l2 = new NomsMap(tr, new MapLeafSequence(ds, tr, [{key: 'e', value: true},
+    const r1 = vwr.writeValue(l1);
+    const l2 = new NomsMap(tr, new MapLeafSequence(vwr, tr, [{key: 'e', value: true},
         {key:'f', value:true}]));
-    const r2 = ds.writeValue(l2);
-    const l3 = new NomsMap(tr, new MapLeafSequence(ds, tr, [{key: 'h', value: false},
+    const r2 = vwr.writeValue(l2);
+    const l3 = new NomsMap(tr, new MapLeafSequence(vwr, tr, [{key: 'h', value: false},
         {key:'i', value:true}]));
-    const r3 = ds.writeValue(l3);
-    const l4 = new NomsMap(tr, new MapLeafSequence(ds, tr, [{key: 'm', value: true},
+    const r3 = vwr.writeValue(l3);
+    const l4 = new NomsMap(tr, new MapLeafSequence(vwr, tr, [{key: 'm', value: true},
         {key:'n', value:false}]));
-    const r4 = ds.writeValue(l4);
+    const r4 = vwr.writeValue(l4);
 
-    const m1 = new NomsMap(tr, new OrderedMetaSequence(ds, tr, [new MetaTuple(r1, 'b', 2),
+    const m1 = new NomsMap(tr, new OrderedMetaSequence(vwr, tr, [new MetaTuple(r1, 'b', 2),
         new MetaTuple(r2, 'f', 2)]));
-    const rm1 = ds.writeValue(m1);
-    const m2 = new NomsMap(tr, new OrderedMetaSequence(ds, tr, [new MetaTuple(r3, 'i', 2),
+    const rm1 = vwr.writeValue(m1);
+    const m2 = new NomsMap(tr, new OrderedMetaSequence(vwr, tr, [new MetaTuple(r3, 'i', 2),
         new MetaTuple(r4, 'n', 2)]));
-    const rm2 = ds.writeValue(m2);
+    const rm2 = vwr.writeValue(m2);
 
-    const c = new NomsMap(tr, new OrderedMetaSequence(ds, tr, [new MetaTuple(rm1, 'f', 4),
+    const c = new NomsMap(tr, new OrderedMetaSequence(vwr, tr, [new MetaTuple(rm1, 'f', 4),
         new MetaTuple(rm2, 'n', 4)]));
     return [c, m1, m2];
   }

js/src/map.js

@@ -2,7 +2,7 @@
 
 import BuzHashBoundaryChecker from './buzhash-boundary-checker.js';
 import RefValue from './ref-value.js';
-import type {ValueReader} from './decode.js';
+import type {ValueReader} from './value-store.js';
 import type {BoundaryChecker, makeChunkFn} from './sequence-chunker.js';
 import type {valueOrPrimitive} from './value.js'; // eslint-disable-line no-unused-vars
 import type {AsyncIterator} from './async-iterator.js';

js/src/meta-sequence.js

@@ -3,7 +3,7 @@
 import BuzHashBoundaryChecker from './buzhash-boundary-checker.js';
 import {sha1Size} from './ref.js';
 import type {BoundaryChecker, makeChunkFn} from './sequence-chunker.js';
-import type {ValueReader} from './decode.js';
+import type {ValueReader} from './value-store.js';
 import type {valueOrPrimitive} from './value.js'; // eslint-disable-line no-unused-vars
 import type {Collection} from './collection.js';
 import {CompoundDesc, makeCompoundType, makeRefType, numberType, valueType} from './type.js';

js/src/ref-value.js

@@ -1,6 +1,6 @@
 // @flow
 
-import type DataStore from './data-store.js';
+import type {ValueReader} from './value-store.js';
 import {describeType} from './encode-human-readable.js';
 import {getRefOfValue} from './get-ref.js';
 import {Kind} from './noms-kind.js';
@@ -30,8 +30,8 @@ export default class RefValue<T: valueOrPrimitive> extends Value {
     return this._type;
   }
 
-  targetValue(store: DataStore): Promise<T> {
-    return store.readValue(this.targetRef);
+  targetValue(vr: ValueReader): Promise<T> {
+    return vr.readValue(this.targetRef);
   }
 
   less(other: Value): boolean {

js/src/sequence.js

@@ -1,6 +1,6 @@
 // @flow
 
-import type {ValueReader} from './decode.js';
+import type {ValueReader} from './value-store.js';
 import {invariant, notNull} from './assert.js';
 import {AsyncIterator} from './async-iterator.js';
 import type {AsyncIteratorResult} from './async-iterator.js';

js/src/set-test.js

@@ -26,6 +26,7 @@ import {newSet, NomsSet, SetLeafSequence} from './set.js';
 import {OrderedSequence} from './ordered-sequence.js';
 import Ref from './ref.js';
 import type {Type} from './type.js';
+import type {ValueReadWriter} from './value-store.js';
 
 const testSetSize = 5000;
 const setOfNRef = 'sha1-5b4cd51d88b3d99e6dafdb1cafb8cec90d5aecdf';
@@ -250,14 +251,14 @@ suite('SetLeaf', () => {
 });
 
 suite('CompoundSet', () => {
-  function build(ds: DataStore, values: Array<string>): NomsSet {
+  function build(vwr: ValueReadWriter, values: Array<string>): NomsSet {
     const tr = makeSetType(stringType);
     assert.isTrue(values.length > 1 && Math.log2(values.length) % 1 === 0);
 
     let tuples = [];
     for (let i = 0; i < values.length; i += 2) {
-      const l = new NomsSet(tr, new SetLeafSequence(ds, tr, [values[i], values[i + 1]]));
-      const r = ds.writeValue(l);
+      const l = new NomsSet(tr, new SetLeafSequence(vwr, tr, [values[i], values[i + 1]]));
+      const r = vwr.writeValue(l);
       tuples.push(new MetaTuple(r, values[i + 1], 2));
     }
 
@@ -265,8 +266,8 @@ suite('CompoundSet', () => {
     while (tuples.length > 1) {
       const next = [];
       for (let i = 0; i < tuples.length; i += 2) {
-        last = new NomsSet(tr, new OrderedMetaSequence(ds, tr, [tuples[i], tuples[i + 1]]));
-        const r = ds.writeValue(last);
+        last = new NomsSet(tr, new OrderedMetaSequence(vwr, tr, [tuples[i], tuples[i + 1]]));
+        const r = vwr.writeValue(last);
         next.push(new MetaTuple(r, tuples[i + 1].value,
                                 tuples[i].numLeaves + tuples[i + 1].numLeaves));
       }

js/src/set.js

@@ -2,7 +2,7 @@
 
 import BuzHashBoundaryChecker from './buzhash-boundary-checker.js';
 import RefValue from './ref-value.js';
-import type {ValueReader} from './decode.js';
+import type {ValueReader} from './value-store.js';
 import type {BoundaryChecker, makeChunkFn} from './sequence-chunker.js';
 import type {valueOrPrimitive, Value} from './value.js'; // eslint-disable-line no-unused-vars
 import {AsyncIterator} from './async-iterator.js';

js/src/specs-test.js

@@ -20,7 +20,7 @@ suite('Specs', () => {
     assert.equal(spec.scheme, 'mem');
     assert.equal(spec.path, '');
     assert.instanceOf(spec.store(), DataStore);
-    assert.instanceOf(spec.store()._bs, BatchStoreAdaptor);
+    assert.instanceOf(spec.store()._vs._bs, BatchStoreAdaptor);
 
     spec = DataStoreSpec.parse('http://foo');
     invariant(spec);
@@ -28,7 +28,7 @@ suite('Specs', () => {
     assert.equal(spec.scheme, 'http');
     assert.equal(spec.path, '//foo');
     assert.instanceOf(spec.store(), DataStore);
-    assert.instanceOf(spec.store()._bs, HttpBatchStore);
+    assert.instanceOf(spec.store()._vs._bs, HttpBatchStore);
 
     spec = DataStoreSpec.parse('https://foo');
     invariant(spec);
@@ -48,7 +48,7 @@ suite('Specs', () => {
     assert.equal(spec.store.path, '');
     let ds = spec.set();
     assert.instanceOf(ds, Dataset);
-    assert.instanceOf(ds.store._bs, BatchStoreAdaptor);
+    assert.instanceOf(ds.store._vs._bs, BatchStoreAdaptor);
 
     spec = DatasetSpec.parse('http://localhost:8000/foo:ds');
     invariant(spec);
@@ -57,7 +57,7 @@ suite('Specs', () => {
     assert.equal(spec.store.path, '//localhost:8000/foo');
     ds = spec.set();
     assert.instanceOf(ds, Dataset);
-    assert.instanceOf(ds.store._bs, HttpBatchStore);
+    assert.instanceOf(ds.store._vs._bs, HttpBatchStore);
   });
 
   test('RefSpec', () => {

js/src/test-util.js

@@ -41,4 +41,4 @@ export async function testRoundTripAndValidate<T: valueOrPrimitive>(v: T,
     assert.strictEqual(v2, v);
   }
   await validateFn(v2);
-}
+}

js/src/value-store.js

@@ -17,10 +17,19 @@ import {invariant, notNull} from './assert.js';
 import {encodeNomsValue} from './encode.js';
 import {describeType, describeTypeOfValue} from './encode-human-readable.js';
 
-export interface Cache<T> {  // eslint-disable-line no-undef
-  entry(ref: Ref): ?CacheEntry<T>;  // eslint-disable-line no-undef
-  get(ref: Ref): ?T;  // eslint-disable-line no-undef
-  add(ref: Ref, size: number, value: T): void;  // eslint-disable-line no-undef
+export interface ValueWriter {
+    writeValue<T: valueOrPrimitive>(v: T, t: ?Type): RefValue<T>
+}
+
+export interface ValueReader {
+  // TODO: This should return Promise<?valueOrPrimitive>
+  readValue(ref: Ref): Promise<any>
+}
+
+export interface ValueReadWriter {
+  // TODO: This should return Promise<?valueOrPrimitive>
+  readValue(ref: Ref): Promise<any>;
+  writeValue<T: valueOrPrimitive>(v: T): RefValue<T>;
 }
 
 export default class ValueStore {
@@ -85,7 +94,13 @@ export default class ValueStore {
   }
 }
 
-export class CacheEntry<T> {
+interface Cache<T> {  // eslint-disable-line no-undef
+  entry(ref: Ref): ?CacheEntry<T>;  // eslint-disable-line no-undef
+  get(ref: Ref): ?T;  // eslint-disable-line no-undef
+  add(ref: Ref, size: number, value: T): void;  // eslint-disable-line no-undef
+}
+
+class CacheEntry<T> {
   size: number;
   value: ?T;