diff --git a/packages/marshal/marshal.js b/packages/marshal/marshal.js index 091cb44dfff..07d59210c10 100644 --- a/packages/marshal/marshal.js +++ b/packages/marshal/marshal.js @@ -156,17 +156,6 @@ export function mustPassByPresence(val) { // ok! } -// This is the equality comparison used by JavaScript's Map and Set -// abstractions, where NaN is the same as NaN and -0 is the same as -// 0. Marshal serializes -0 as zero, so the semantics of our distributed -// object system does not distinguish 0 from -0. -// -// `sameValueZero` is the EcmaScript spec name for this equality comparison, -// but TODO we need a better name for the API. -export function sameValueZero(x, y) { - return x === y || Object.is(x, y); -} - // How would val be passed? For primitive values, the answer is // * 'null' for null // * throwing an error for a symbol, whether registered or not. diff --git a/packages/same-structure/src/sameStructure.js b/packages/same-structure/src/sameStructure.js index 23aadf74720..25ae855d28f 100644 --- a/packages/same-structure/src/sameStructure.js +++ b/packages/same-structure/src/sameStructure.js @@ -1,6 +1,7 @@ import harden from '@agoric/harden'; -import { sameValueZero, passStyleOf } from '@agoric/marshal'; +import { passStyleOf } from '@agoric/marshal'; import { assert, details, openDetail } from '@agoric/assert'; +import { sameKey } from '../../store/src/store'; // Shim of Object.fromEntries from // https://github.com/tc39/proposal-object-from-entries/blob/master/polyfill.js @@ -114,7 +115,7 @@ function sameStructure(left, right) { case 'symbol': case 'bigint': case 'presence': { - return sameValueZero(left, right); + return sameKey(left, right); } case 'copyRecord': case 'copyArray': { @@ -193,7 +194,7 @@ function mustBeSameStructureInternal(left, right, message, path) { case 'symbol': case 'bigint': case 'presence': { - if (!sameValueZero(left, right)) { + if (!sameKey(left, right)) { complain('different'); } break; diff --git a/packages/store/package.json b/packages/store/package.json index c8570ea3e5f..dcad4d550b8 100644 --- a/packages/store/package.json +++ b/packages/store/package.json @@ -8,7 +8,7 @@ }, "scripts": { "build": "exit 0", - "test": "exit 0", + "test": "tape -r esm test/**/test*.js", "lint-fix": "eslint --fix '**/*.js'", "lint-check": "eslint '**/*.js'", "lint-fix-jessie": "eslint -c '.eslintrc-jessie.js' --fix '**/*.js'", @@ -29,7 +29,8 @@ "homepage": "https://github.com/Agoric/agoric-sdk#readme", "dependencies": { "@agoric/harden": "^0.0.4", - "@agoric/assert": "^0.0.1" + "@agoric/assert": "^0.0.1", + "@agoric/weak-store": "^0.0.1" }, "devDependencies": { "esm": "^3.2.25", diff --git a/packages/store/src/store.js b/packages/store/src/store.js index 820c4f43114..bf1c19717f3 100644 --- a/packages/store/src/store.js +++ b/packages/store/src/store.js @@ -2,43 +2,114 @@ import harden from '@agoric/harden'; import { assert, details, openDetail } from '@agoric/assert'; + +// `sameKey` is the equality comparison used by JavaScript's Map and Set +// abstractions to compare their keys. Its internal JavaScript spec name +// is "SameValueZero". It forms a proper equivalence class that is less +// precise than `Object.is`. Unlike `===`, `sameKey` is an equivalence +// class because `sameKey(NaN, NaN)`. `sameKey` is more precise than +// `Object.is` because `!sameKey(-0, 0)`. +// +// The simple `makeStore` exported by this module makes stores that +// also use `sameKey` equality to compare their keys. +// +// Marshal serializes -0 as zero, so the semantics of our distributed +// object system also does not distinguish 0 from -0. +// +// `sameValueZero` is the EcmaScript spec name for this equality comparison. +// Unlike `===`, `sameKey` is an equivalence class, since +export const sameKey = (a, b) => a === b || Object.is(a, b); + /** * Distinguishes between adding a new key (init) and updating or * referencing a key (get, set, delete). * - * `init` is only allowed if the key does not already exist. `Get`, + * `init` is only allowed if the key does not already exist. `get`, * `set` and `delete` are only allowed if the key does already exist. + * For any of these methods, an expression like `store.get`, + * extracting the `get` method from a store using dot (`.`), extracts + * a function that is effectively bound to that store, which can be + * passed and used elsewhere. + * * @param {string} keyName - the column name for the key */ function makeStore(keyName = 'key') { - const store = new Map(); - const assertKeyDoesNotExist = key => + const map = new Map(); + const assertKeyNotBound = key => assert( - !store.has(key), + !map.has(key), details`${openDetail(keyName)} already registered: ${key}`, ); - const assertKeyExists = key => - assert(store.has(key), details`${openDetail(keyName)} not found: ${key}`); + const assertKeyBound = key => + assert(map.has(key), details`${openDetail(keyName)} not found: ${key}`); + + // /////// Methods ///////// + + const has = key => map.has(key); + const init = (key, value) => { + assertKeyNotBound(key); + map.set(key, value); + }; + const get = key => { + assertKeyBound(key); + return map.get(key); + }; + const set = (key, value) => { + assertKeyBound(key); + map.set(key, value); + }; + const deleteIt = key => { + assertKeyBound(key); + map.delete(key); + }; + const keys = () => [...map.keys()]; + const values = () => [...map.values()]; + const entries = () => [...map.entries()]; + + const diverge = () => { + const result = makeStore(keyName); + for (const [k, v] of entries()) { + result.init(k, v); + } + return result; + }; + + // eslint-disable-next-line no-use-before-define + const readOnly = () => readOnlyStore; + + const snapshot = () => { + const snapshotStore = harden({ + ...diverge().readOnly(), + snapshot: () => snapshotStore, + }); + return snapshotStore; + }; + + const readOnlyStore = harden({ + diverge, + readOnly, + snapshot, + has, + get, + keys, + values, + entries, + [Symbol.iterator]: entries, + }); + return harden({ - has: key => store.has(key), - init: (key, value) => { - assertKeyDoesNotExist(key); - store.set(key, value); - }, - get: key => { - assertKeyExists(key); - return store.get(key); - }, - set: (key, value) => { - assertKeyExists(key); - store.set(key, value); - }, - delete: key => { - assertKeyExists(key); - store.delete(key); - }, - keys: () => Array.from(store.keys()), - values: () => Array.from(store.values()), + diverge, + readOnly, + snapshot, + has, + init, + get, + set, + delete: deleteIt, + keys, + values, + entries, + [Symbol.iterator]: entries, }); } harden(makeStore); diff --git a/packages/store/src/trieStore.js b/packages/store/src/trieStore.js new file mode 100644 index 00000000000..a2433d0042c --- /dev/null +++ b/packages/store/src/trieStore.js @@ -0,0 +1,231 @@ +// Copyright (C) 2019 Agoric, under Apache license 2.0 + +import harden from '@agoric/harden'; +import { assert, details, openDetail } from '@agoric/assert'; +import makeWeakStore from '@agoric/weak-store'; +import makeStrongStore, { sameKey } from './store'; + +// The key equality used by the stores exported by this module +// is to compare to arrays element by element using normal +// JavaScript map equality. +// ```js +// sameTrieKey([NaN, 0], [NaN, -0]); +// ``` +export const sameTrieKey = (a, b) => + a.length === b.length && a.every((v, i) => sameKey(v, b[i])); + +// The Pumpkin must not escape. It must be distinct from any value that +// could be passed into this module. +const Pumpkin = harden({}); + +// Not intended for export; just for internal use. +// `enumerable` should only be true if the `makeStore` argument +// makes enumerable stores. +function makeTrieStoreMaker(makeStore, enumerable = false) { + /** + * A trieStore is a *store* (as defined in store.js) whose keys are arrays + * that are compared and looked up based on element-by-element equality. + * Internally it uses a trie made of a tree of stores. + * + * @param {string} keyName - the column name for the key + */ + function makeTrieStore(keyName = 'key') { + const root = makeStore(keyName); + + // This caches lastStore keyed by the key *identity*. + // That way, once a given key is dropped, the cache entry gets collected. + // If the same key identity is reused, such as by a .has followed by a + // .get, it's fast. Should have no observable effect. + // + // Since the cache is *only* for speed, if it actually makes things slower + // because of WeakMap collection memory pressure, get rid of the cache. + const lastStoreCache = new WeakMap(); + + // If the key is already bound, return the last store in the trie which + // binds the Pumpkin to the key's current value. Otherwise, return + // undefined. + function finalStoreIfKeyBound(key) { + let cursor; + if (lastStoreCache.has(key) === key) { + cursor = lastStoreCache.get(key); + } else { + assert(Array.isArray(key), details`trie key must be an array: ${key}`); + + cursor = root; + for (const element of key) { + // A "can never happen" that should abort rather than throw + assert(element !== Pumpkin, details`internal: Pumpkin leaked!`); + if (!cursor.has(element)) { + return undefined; + } + // cannot error because of .has test + cursor = cursor.get(element); + } + lastStoreCache.set(key, cursor); + } + if (!cursor.has(Pumpkin)) { + return undefined; + } + return cursor; + } + + // Assert that the key is already bound, like assertKeyBound of store.js. + // Return the last store in the trie which binds the Pumpkin to the key's + // current value. + function finalStoreAssertKeyBound(key) { + const result = finalStoreIfKeyBound(key); + assert( + result !== undefined, + details`${openDetail(keyName)} not found: ${key}`, + ); + return result; + } + + // Assert that the key is not already bound, like assertKeyNotBound of + // store.js. + // Return the last store in the trie for this key, making it if + // necessary, in which there is not yet a binding from the Pumpkin to + // this key's value. + function finalStoreAssertKeyNotBound(key) { + let cursor; + if (lastStoreCache.has(key) === key) { + cursor = lastStoreCache.get(key); + } else { + assert(Array.isArray(key), details`trie key must be an array: ${key}`); + + cursor = root; + for (const element of key) { + // A "can never happen" that should abort rather than throw + assert(element !== Pumpkin, details`internal: Pumpkin leaked!`); + if (cursor.has(element)) { + // cannot error because of .has test + cursor = cursor.get(element); + } else { + const nextCursor = makeStore(keyName); + // cannot error because of .has test + cursor.init(element, nextCursor); + cursor = nextCursor; + } + } + lastStoreCache.set(key, cursor); + } + assert( + !cursor.has(Pumpkin), + details`${openDetail(keyName)} already registered: ${key}`, + ); + return cursor; + } + + // /////// Methods ///////// + + const has = key => finalStoreIfKeyBound(key) !== undefined; + const init = (key, value) => { + const lastStore = finalStoreAssertKeyNotBound(key); + lastStore.init(Pumpkin, value); + }; + const get = key => { + const lastStore = finalStoreAssertKeyBound(key); + return lastStore.get(Pumpkin); + }; + const set = (key, value) => { + const lastStore = finalStoreAssertKeyBound(key); + lastStore.set(Pumpkin, value); + }; + const deleteIt = key => { + const lastStore = finalStoreAssertKeyBound(key); + lastStore.delete(Pumpkin); + }; + + // eslint-disable-next-line no-use-before-define + const readOnly = () => readOnlyTrieStore; + + let readOnlyEnumerators = {}; + if (enumerable) { + const entries = () => { + const result = []; + const walk = (prefix, node) => { + for (const [k, v] of node) { + if (k === Pumpkin) { + result.push([prefix, v]); + } else { + walk([...prefix, k], v); + } + } + }; + walk([], root); + return result; + }; + const keys = () => entries.map(([k, _]) => k); + const values = () => entries.map(([_, v]) => v); + + const diverge = () => { + const result = makeTrieStore(keyName); + for (const [k, v] of entries()) { + result.init(k, v); + } + return result; + }; + + const snapshot = () => { + const snapshotStore = harden({ + ...diverge().readOnly(), + snapshot: () => snapshotStore, + }); + return snapshotStore; + }; + + readOnlyEnumerators = { + diverge, + snapshot, + keys, + values, + entries, + [Symbol.iterator]: entries, + }; + } + + const readOnlyTrieStore = harden({ + readOnly, + has, + get, + ...readOnlyEnumerators, + }); + + return harden({ + readOnly, + has, + init, + get, + set, + delete: deleteIt, + ...readOnlyEnumerators, + }); + } + harden(makeTrieStore); + return makeTrieStore; +} + +export const makeStrongTrieStore = makeTrieStoreMaker(makeStrongStore, true); +// Is this ever useful? +// export const makeWeakTrieStore = makeTrieStoreMaker(makeWeakStore, false); + +// A simple store that holds objects weakly and everything else strongly. +// Beware of memory leaks. +function makeMixedStore(keyName = 'key') { + const strongStore = makeStrongStore(keyName); + const weakStore = makeWeakStore(keyName); + const storeFor = key => (Object(key) === key ? weakStore : strongStore); + + return harden({ + has: key => storeFor(key).has(key), + init: (key, value) => storeFor(key).init(key, value), + get: key => storeFor(key).get(key), + set: (key, value) => storeFor(key).set(key, value), + delete: key => storeFor(key).delete(key), + }); +} + +// A trieStore in which each element, if it is an object, is held weakly. +// If the element disappears, so does the subtree rooted in that element. +const makeTrieStore = makeTrieStoreMaker(makeMixedStore, false); +export default makeTrieStore; diff --git a/packages/store/test/test-store.js b/packages/store/test/test-store.js new file mode 100644 index 00000000000..da23eceadb8 --- /dev/null +++ b/packages/store/test/test-store.js @@ -0,0 +1,43 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { test } from 'tape-promise/tape'; +import makeStore from '../src/store'; +import { throwsAndLogs } from '../../assert/test/throwsAndLogs'; + +export const testAStore = (t, makeAStore, key) => { + const store = makeAStore('fookey'); + t.equal(store.has(key), false); + throwsAndLogs(t, () => store.get(key), /fookey not found: \(a.*\)/, [ + ['log', 'FAILED ASSERTION false'], + ['error', '', 'fookey', ' not found: ', key, ''], + ]); + store.init(key, 8); + t.equal(store.has(key), true); + t.equal(store.get(key), 8); + store.set(key, 9); + t.equal(store.get(key), 9); + throwsAndLogs( + t, + () => store.init(key, 7), + /fookey already registered: \(a.*\)/, + [ + ['log', 'FAILED ASSERTION false'], + ['error', '', 'fookey', ' already registered: ', key, ''], + ], + ); + store.delete(key); + t.equal(store.has(key), false); + store.init(key, 5); + t.equal(store.has(key), true); + t.equal(store.get(key), 5); +}; + +test('store', t => { + try { + testAStore(t, makeStore, 'x'); + } catch (e) { + console.log('unexpected exception', e); + t.assert(false, e); + } finally { + t.end(); + } +}); diff --git a/packages/store/test/test-trieStore.js b/packages/store/test/test-trieStore.js new file mode 100644 index 00000000000..10b5247e56a --- /dev/null +++ b/packages/store/test/test-trieStore.js @@ -0,0 +1,15 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { test } from 'tape-promise/tape'; +import makeTrieStore from '../src/trieStore'; +import { testAStore } from './test-store'; + +test('trieStore', t => { + try { + testAStore(t, makeTrieStore, [NaN, -0, {}]); + } catch (e) { + console.log('unexpected exception', e); + t.assert(false, e); + } finally { + t.end(); + } +}); diff --git a/packages/weak-store/package.json b/packages/weak-store/package.json index cadc9f42759..1ea4258b759 100644 --- a/packages/weak-store/package.json +++ b/packages/weak-store/package.json @@ -8,7 +8,7 @@ }, "scripts": { "build": "exit 0", - "test": "exit 0", + "test": "tape -r esm test/**/test*.js", "lint-fix": "eslint --fix '**/*.js'", "lint-check": "eslint '**/*.js'", "lint-fix-jessie": "eslint -c '.eslintrc-jessie.js' --fix '**/*.js'", diff --git a/packages/weak-store/src/weakStore.js b/packages/weak-store/src/weakStore.js index da6f6522eee..5bc5aff215b 100644 --- a/packages/weak-store/src/weakStore.js +++ b/packages/weak-store/src/weakStore.js @@ -10,34 +10,53 @@ import { assert, details, openDetail } from '@agoric/assert'; * `set` and `delete` are only allowed if the key does already exist. * @param {string} keyName - the column name for the key */ -function makeStore(keyName = 'key') { +function makeWeakStore(keyName = 'key') { const wm = new WeakMap(); - const assertKeyDoesNotExist = key => + const assertKeyNotBound = key => assert( !wm.has(key), details`${openDetail(keyName)} already registered: ${key}`, ); - const assertKeyExists = key => + const assertKeyBound = key => assert(wm.has(key), details`${openDetail(keyName)} not found: ${key}`); + + // /////// Methods ///////// + + const has = key => wm.has(key); + const init = (key, value) => { + assertKeyNotBound(key); + wm.set(key, value); + }; + const get = key => { + assertKeyBound(key); + return wm.get(key); + }; + const set = (key, value) => { + assertKeyBound(key); + wm.set(key, value); + }; + const deleteIt = key => { + assertKeyBound(key); + wm.delete(key); + }; + + // eslint-disable-next-line no-use-before-define + const readOnly = () => readOnlyWeakStore; + + const readOnlyWeakStore = harden({ + readOnly, + has, + get, + }); + return harden({ - has: key => wm.has(key), - init: (key, value) => { - assertKeyDoesNotExist(key); - wm.set(key, value); - }, - get: key => { - assertKeyExists(key); - return wm.get(key); - }, - set: (key, value) => { - assertKeyExists(key); - wm.set(key, value); - }, - delete: key => { - assertKeyExists(key); - wm.delete(key); - }, + readOnly, + has, + init, + get, + set, + delete: deleteIt, }); } -harden(makeStore); -export default makeStore; +harden(makeWeakStore); +export default makeWeakStore; diff --git a/packages/weak-store/test/test-weakStore.js b/packages/weak-store/test/test-weakStore.js new file mode 100644 index 00000000000..0a9c01a7de6 --- /dev/null +++ b/packages/weak-store/test/test-weakStore.js @@ -0,0 +1,15 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { test } from 'tape-promise/tape'; +import makeWeakStore from '../src/weakStore'; +import { testAStore } from '../../store/test/test-store'; + +test('weakStore', t => { + try { + testAStore(t, makeWeakStore, {}); + } catch (e) { + console.log('unexpected exception', e); + t.assert(false, e); + } finally { + t.end(); + } +});