From fd864bd9df1dc0e8d2a4550cb6abcbbbdda6a64e Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sat, 20 Aug 2022 19:26:54 -0700 Subject: [PATCH 1/5] fix: prepare for binary marshalling --- packages/marshal/package.json | 1 + packages/marshal/src/encodePassable.js | 434 +++++++++++++++ packages/marshal/src/rankOrder.js | 545 +++++++++++++++++++ packages/marshal/test/test-encodePassable.js | 129 +++++ packages/marshal/test/test-rankOrder.js | 377 +++++++++++++ yarn.lock | 19 + 6 files changed, 1505 insertions(+) create mode 100644 packages/marshal/src/encodePassable.js create mode 100644 packages/marshal/src/rankOrder.js create mode 100644 packages/marshal/test/test-encodePassable.js create mode 100644 packages/marshal/test/test-rankOrder.js diff --git a/packages/marshal/package.json b/packages/marshal/package.json index a8947f6781..845663f497 100644 --- a/packages/marshal/package.json +++ b/packages/marshal/package.json @@ -45,6 +45,7 @@ "@endo/init": "^0.5.51", "@endo/lockdown": "^0.1.23", "@endo/ses-ava": "^0.2.35", + "@fast-check/ava": "^1.0.1", "ava": "^3.12.1", "c8": "^7.7.3" }, diff --git a/packages/marshal/src/encodePassable.js b/packages/marshal/src/encodePassable.js new file mode 100644 index 0000000000..9993b21a03 --- /dev/null +++ b/packages/marshal/src/encodePassable.js @@ -0,0 +1,434 @@ +/* eslint-disable no-bitwise */ +// @ts-check +import { getTag } from './helpers/passStyle-helpers.js'; +import { makeTagged } from './makeTagged.js'; +import { passStyleOf } from './passStyleOf.js'; +import { assertRecord } from './typeGuards.js'; +import { + nameForPassableSymbol, + passableSymbolForName, +} from './helpers/symbol.js'; +import { recordParts } from './rankOrder.js'; +import { ErrorHelper } from './helpers/error.js'; + +/** @typedef {import('./types.js').Passable} Passable */ +/** @typedef {import('./types.js').Remotable} Remotable */ + +const { details: X, quote: q } = assert; +const { is, fromEntries } = Object; + +export const zeroPad = (n, size) => { + const nStr = `${n}`; + assert(nStr.length <= size); + const str = `00000000000000000000${nStr}`; + const result = str.substring(str.length - size); + assert(result.length === size); + return result; +}; +harden(zeroPad); + +// This is the JavaScript analog to a C union: a way to map between a float as a +// number and the bits that represent the float as a buffer full of bytes. Note +// that the mutation of static state here makes this invalid Jessie code, but +// doing it this way saves the nugatory and gratuitous allocations that would +// happen every time you do a conversion -- and in practical terms it's safe +// because we put the value in one side and then immediately take it out the +// other; there is no actual state retained in the classic sense and thus no +// re-entrancy issue. +const asNumber = new Float64Array(1); +const asBits = new BigUint64Array(asNumber.buffer); + +// JavaScript numbers are encoded by outputting the base-16 +// representation of the binary value of the underlying IEEE floating point +// representation. For negative values, all bits of this representation are +// complemented prior to the base-16 conversion, while for positive values, the +// sign bit is complemented. This ensures both that negative values sort before +// positive values and that negative values sort according to their negative +// magnitude rather than their positive magnitude. This results in an ASCII +// encoding whose lexicographic sort order is the same as the numeric sort order +// of the corresponding numbers. + +// TODO Choose the same canonical NaN encoding that cosmWasm and ewasm chose. +const CanonicalNaNBits = 'fff8000000000000'; + +const encodeBinary64 = n => { + // Normalize -0 to 0 and NaN to a canonical encoding + if (is(n, -0)) { + n = 0; + } else if (is(n, NaN)) { + return `f${CanonicalNaNBits}`; + } + asNumber[0] = n; + let bits = asBits[0]; + if (n < 0) { + bits ^= 0xffffffffffffffffn; + } else { + bits ^= 0x8000000000000000n; + } + return `f${zeroPad(bits.toString(16), 16)}`; +}; + +const decodeBinary64 = encoded => { + assert(encoded.startsWith('f'), X`Encoded number expected: ${encoded}`); + let bits = BigInt(`0x${encoded.substring(1)}`); + if (encoded[1] < '8') { + bits ^= 0xffffffffffffffffn; + } else { + bits ^= 0x8000000000000000n; + } + asBits[0] = bits; + const result = asNumber[0]; + assert(!is(result, -0), X`Unexpected negative zero: ${encoded}`); + return result; +}; + +// JavaScript bigints are encoded using a variant of Elias delta coding, with an +// initial component for the length of the digit count as a unary string, a +// second component for the decimal digit count, and a third component for the +// decimal digits preceded by a gratuitous separating colon. +// To ensure that the lexicographic sort order of encoded values matches the +// numeric sort order of the corresponding numbers, the characters of the unary +// prefix are different for negative values (type "n" followed by any number of +// "#"s [which sort before decimal digits]) vs. positive and zero values (type +// "p" followed by any number of "~"s [which sort after decimal digits]) and +// each decimal digit of the encoding for a negative value is replaced with its +// ten's complement (so that negative values of the same scale sort by +// *descending* absolute value). +const encodeBigInt = n => { + const abs = n < 0n ? -n : n; + const nDigits = abs.toString().length; + const lDigits = nDigits.toString().length; + if (n < 0n) { + return `n${ + // A "#" for each digit beyond the first + // in the decimal *count* of decimal digits. + '#'.repeat(lDigits - 1) + }${ + // The ten's complement of the count of digits. + (10 ** lDigits - nDigits).toString().padStart(lDigits, '0') + }:${ + // The ten's complement of the digits. + (10n ** BigInt(nDigits) + n).toString().padStart(nDigits, '0') + }`; + } else { + return `p${ + // A "~" for each digit beyond the first + // in the decimal *count* of decimal digits. + '~'.repeat(lDigits - 1) + }${ + // The count of digits. + nDigits + }:${ + // The digits. + n + }`; + } +}; + +const decodeBigInt = encoded => { + const typePrefix = encoded[0]; + let rem = encoded.slice(1); + typePrefix === 'p' || + typePrefix === 'n' || + assert.fail(X`Encoded bigint expected: ${encoded}`); + + const lDigits = rem.search(/[0-9]/) + 1; + assert(lDigits >= 1, X`Digit count expected: ${encoded}`); + rem = rem.slice(lDigits - 1); + + assert(rem.length >= lDigits, X`Complete digit count expected: ${encoded}`); + const snDigits = rem.slice(0, lDigits); + rem = rem.slice(lDigits); + /^[0-9]+$/.test(snDigits) || + assert.fail(X`Decimal digit count expected: ${encoded}`); + let nDigits = parseInt(snDigits, 10); + if (typePrefix === 'n') { + // TODO Assert to reject forbidden encodings + // like "n0:" and "n00:…" and "n91:…" through "n99:…"? + nDigits = 10 ** lDigits - nDigits; + } + + assert(rem.startsWith(':'), X`Separator expected: ${encoded}`); + rem = rem.slice(1); + rem.length === nDigits || + assert.fail(X`Fixed-length digit sequence expected: ${encoded}`); + let n = BigInt(rem); + if (typePrefix === 'n') { + // TODO Assert to reject forbidden encodings + // like "n9:0" and "n8:00" and "n8:91" through "n8:99"? + n = -(10n ** BigInt(nDigits) - n); + } + + return n; +}; + +// `'\u0000'` is the terminator after elements. +// `'\u0001'` is the backslash-like escape character, for +// escaping both of these characters. + +const encodeArray = (array, encodePassable) => { + const chars = ['[']; + for (const element of array) { + const enc = encodePassable(element); + for (const c of enc) { + if (c === '\u0000' || c === '\u0001') { + chars.push('\u0001'); + } + chars.push(c); + } + chars.push('\u0000'); + } + return chars.join(''); +}; + +const decodeArray = (encoded, decodePassable) => { + assert(encoded.startsWith('['), X`Encoded array expected: ${encoded}`); + const elements = []; + const elemChars = []; + for (let i = 1; i < encoded.length; i += 1) { + const c = encoded[i]; + if (c === '\u0000') { + const encodedElement = elemChars.join(''); + elemChars.length = 0; + const element = decodePassable(encodedElement); + elements.push(element); + } else if (c === '\u0001') { + i += 1; + assert(i < encoded.length, X`unexpected end of encoding ${encoded}`); + const c2 = encoded[i]; + c2 === '\u0000' || + c2 === '\u0001' || + assert.fail(X`Unexpected character after u0001 escape: ${c2}`); + elemChars.push(c2); + } else { + elemChars.push(c); + } + } + assert(elemChars.length === 0, X`encoding terminated early: ${encoded}`); + return harden(elements); +}; + +const encodeRecord = (record, encodePassable) => { + const [names, values] = recordParts(record); + return `(${encodeArray(harden([names, values]), encodePassable)}`; +}; + +const decodeRecord = (encoded, decodePassable) => { + assert(encoded.startsWith('(')); + const keysvals = decodeArray(encoded.substring(1), decodePassable); + assert(keysvals.length === 2, X`expected keys,values pair: ${encoded}`); + const [keys, vals] = keysvals; + assert( + passStyleOf(keys) === 'copyArray' && + passStyleOf(vals) === 'copyArray' && + keys.length === vals.length && + keys.every(key => typeof key === 'string'), + X`not a valid record encoding: ${encoded}`, + ); + const entries = keys.map((key, i) => [key, vals[i]]); + const record = harden(fromEntries(entries)); + assertRecord(record, 'decoded record'); + return record; +}; + +const encodeTagged = (tagged, encodePassable) => + `:${encodeArray(harden([getTag(tagged), tagged.payload]), encodePassable)}`; + +const decodeTagged = (encoded, decodePassable) => { + assert(encoded.startsWith(':')); + const tagpayload = decodeArray(encoded.substring(1), decodePassable); + assert(tagpayload.length === 2, X`expected tag,payload pair: ${encoded}`); + const [tag, payload] = tagpayload; + passStyleOf(tag) === 'string' || + assert.fail(X`not a valid tagged encoding: ${encoded}`); + return makeTagged(tag, payload); +}; + +/** + * @typedef {object} EncodeOptions + * @property {( + * remotable: Remotable, + * encodeRecur: (p: Passable) => string, + * ) => string} [encodeRemotable] + * @property {( + * promise: Promise, + * encodeRecur: (p: Passable) => string, + * ) => string} [encodePromise] + * @property {( + * error: Error, + * encodeRecur: (p: Passable) => string, + * ) => string} [encodeError] + */ + +/** + * @param {EncodeOptions} [encodeOptions] + * @returns {(passable: Passable) => string} + */ +// `yarn lint` complains here but not for equivalent code in agoric-sdk. +// Also, vscode does not complain. Hence we're using at-ts-ignore rather than +// at-ts-expect-error. Using at-ts-ignore should also generate a complaint +// that we should be using at-expect-error, where we would normally need +// to suppress that error as well. However, perhaps that second error currently +// happens only in agoric-sdk, but not yet in endo. TODO figure out and fix. +// @ts-ignore +export const makeEncodePassable = ({ + encodeRemotable = (rem, _) => assert.fail(X`remotable unexpected: ${rem}`), + encodePromise = (prom, _) => assert.fail(X`promise unexpected: ${prom}`), + encodeError = (err, _) => assert.fail(X`error unexpected: ${err}`), +} = {}) => { + const encodePassable = passable => { + if (ErrorHelper.canBeValid(passable, x => x)) { + return encodeError(passable, encodePassable); + } + const passStyle = passStyleOf(passable); + switch (passStyle) { + case 'null': { + return 'v'; + } + case 'undefined': { + return 'z'; + } + case 'number': { + return encodeBinary64(passable); + } + case 'string': { + return `s${passable}`; + } + case 'boolean': { + return `b${passable}`; + } + case 'bigint': { + return encodeBigInt(passable); + } + case 'remotable': { + const result = encodeRemotable(passable, encodePassable); + result.startsWith('r') || + assert.fail( + X`internal: Remotable encoding must start with "r": ${result}`, + ); + return result; + } + case 'error': { + const result = encodeError(passable, encodePassable); + result.startsWith('!') || + assert.fail( + X`internal: Error encoding must start with "!": ${result}`, + ); + return result; + } + case 'promise': { + const result = encodePromise(passable, encodePassable); + result.startsWith('?') || + assert.fail( + X`internal: Promise encoding must start with "?": ${result}`, + ); + return result; + } + case 'symbol': { + return `y${nameForPassableSymbol(passable)}`; + } + case 'copyArray': { + return encodeArray(passable, encodePassable); + } + case 'copyRecord': { + return encodeRecord(passable, encodePassable); + } + case 'tagged': { + return encodeTagged(passable, encodePassable); + } + default: { + assert.fail( + X`a ${q(passStyle)} cannot be used as a collection passable`, + ); + } + } + }; + return harden(encodePassable); +}; +harden(makeEncodePassable); + +/** + * @typedef {object} DecodeOptions + * @property {( + * encodedRemotable: string, + * decodeRecur: (e: string) => Passable + * ) => Remotable} [decodeRemotable] + * @property {( + * encodedPromise: string, + * decodeRecur: (e: string) => Passable + * ) => Promise} [decodePromise] + * @property {( + * encodedError: string, + * decodeRecur: (e: string) => Passable + * ) => Error} [decodeError] + */ + +/** + * @param {DecodeOptions} [decodeOptions] + * @returns {(encoded: string) => Passable} + */ +// `yarn lint` complains here but not for equivalent code in agoric-sdk. +// Also, vscode does not complain. Hence we're using at-ts-ignore rather than +// at-ts-expect-error. Using at-ts-ignore should also generate a complaint +// that we should be using at-expect-error, where we would normally need +// to suppress that error as well. However, perhaps that second error currently +// happens only in agoric-sdk, but not yet in endo. TODO figure out and fix. +// @ts-ignore +export const makeDecodePassable = ({ + decodeRemotable = (rem, _) => assert.fail(X`remotable unexpected: ${rem}`), + decodePromise = (prom, _) => assert.fail(X`promise unexpected: ${prom}`), + decodeError = (err, _) => assert.fail(X`error unexpected: ${err}`), +} = {}) => { + const decodePassable = encoded => { + switch (encoded[0]) { + case 'v': { + return null; + } + case 'z': { + return undefined; + } + case 'f': { + return decodeBinary64(encoded); + } + case 's': { + return encoded.substring(1); + } + case 'b': { + return encoded.substring(1) !== 'false'; + } + case 'n': + case 'p': { + return decodeBigInt(encoded); + } + case 'r': { + return decodeRemotable(encoded, decodePassable); + } + case '?': { + return decodePromise(encoded, decodePassable); + } + case '!': { + return decodeError(encoded, decodePassable); + } + case 'y': { + return passableSymbolForName(encoded.substring(1)); + } + case '[': { + return decodeArray(encoded, decodePassable); + } + case '(': { + return decodeRecord(encoded, decodePassable); + } + case ':': { + return decodeTagged(encoded, decodePassable); + } + default: { + assert.fail(X`invalid database key: ${encoded}`); + } + } + }; + return harden(decodePassable); +}; +harden(makeDecodePassable); + +export const isEncodedRemotable = encoded => encoded[0] === 'r'; +harden(isEncodedRemotable); diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js new file mode 100644 index 0000000000..6b462c569f --- /dev/null +++ b/packages/marshal/src/rankOrder.js @@ -0,0 +1,545 @@ +// @ts-check +import { getTag } from './helpers/passStyle-helpers.js'; +import { passStyleOf } from './passStyleOf.js'; +import { assertRecord } from './typeGuards.js'; +import { nameForPassableSymbol } from './helpers/symbol.js'; + +/** @typedef {import('./types.js').Passable} Passable */ +/** @typedef {import('./types.js').PassStyle} PassStyle */ + +const { details: X, quote: q } = assert; +const { fromEntries, entries, setPrototypeOf, is } = Object; +const { ownKeys } = Reflect; + +/** + * @typedef {-1 | 0 | 1} RankComparison + * The result of a `RankCompare` function that defines a rank-order, i.e., + * a total preorder in which different elements are always comparable but + * can be tied for the same rank. See `RankCompare`. + */ + +/** + * @callback RankCompare + * Returns `-1`, `0`, or `1` depending on whether the rank of `left` + * is before, tied-with, or after the rank of `right`. + * + * This comparison function is valid as argument to + * `Array.prototype.sort`. This is sometimes described as a "total order" + * but, depending on your definitions, this is technically incorrect because + * it may return `0` to indicate that two distinguishable elements such as + * `-0` and `0` are tied (i.e., are in the same equivalence class + * for the purposes of this ordering). If each such equivalence class is + * a *rank* and ranks are disjoint, then this "rank order" is a + * true total order over these ranks. In mathematics this goes by several + * other names such as "total preorder". + * + * This function establishes a total rank order over all passables. + * To do so it makes arbitrary choices, such as that all strings + * are after all numbers. Thus, this order is not intended to be + * used directly as a comparison with useful semantics. However, it must be + * closely enough related to such comparisons to aid in implementing + * lookups based on those comparisons. For example, in order to get a total + * order among ranks, we put `NaN` after all other JavaScript "number" values + * (i.e., IEEE 754 floating-point values). But otherwise, we rank JavaScript + * numbers by signed magnitude, with `0` and `-0` tied. A semantically useful + * ordering would also compare magnitudes, and so agree with the rank ordering + * of all values other than `NaN`. An array sorted by rank would enable range + * queries by magnitude. + * @param {Passable} left + * @param {Passable} right + * @returns {RankComparison} + */ + +/** + * @typedef {object} RankComparatorKit + * @property {RankCompare} comparator + * @property {RankCompare} antiComparator + */ + +/** + * @typedef {RankCompare} FullCompare + * A `FullCompare` function satisfies all the invariants stated below for + * `RankCompare`'s relation with KeyCompare. + * In addition, its equality is as precise as the `KeyCompare` + * comparison defined below, in that, for all Keys `x` and `y`, + * `FullCompare(x, y) === 0` iff `KeyCompare(x, y) === 0`. + * + * For non-keys a `FullCompare` should be exactly as imprecise as + * `RankCompare`. For example, both will treat all errors as in the same + * equivalence class. Both will treat all promises as in the same + * equivalence class. Both will order taggeds the same way, which is admittedly + * weird, as some taggeds will be considered keys and other taggeds will be + * considered non-keys. + */ + +/** + * @typedef {object} FullComparatorKit + * @property {FullCompare} comparator + * @property {FullCompare} antiComparator + */ + +/** + * @typedef {[string, string]} RankCover + */ + +/** + * @typedef {[number, number]} IndexCover + */ + +/** + * 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. + * + * @param {any} x + * @param {any} y + * @returns {boolean} + */ +const sameValueZero = (x, y) => x === y || is(x, y); + +/** + * @type {[PassStyle, RankCover][]} + */ +const PassStyleRankAndCover = harden([ + /* ! */ ['error', ['!', '!~']], + /* ( */ ['copyRecord', ['(', '(~']], + /* : */ ['tagged', [':', ':~']], + /* ? */ ['promise', ['?', '?~']], + /* [ */ ['copyArray', ['[', '[~']], + /* b */ ['boolean', ['b', 'b~']], + /* f */ ['number', ['f', 'f~']], + /* np */ ['bigint', ['n', 'p~']], + /* r */ ['remotable', ['r', 'r~']], + /* s */ ['string', ['s', 't']], + /* v */ ['null', ['v', 'v~']], + /* y */ ['symbol', ['y', 'z']], + /* z */ ['undefined', ['z', '{']], + /* | remotable->ordinal mapping prefix: This is not used in covers but it is + reserved from the same set of strings. Note that the prefix is > any + prefix used by any cover so that ordinal mapping keys are always outside + the range of valid collection entry keys. */ +]); + +const PassStyleRank = fromEntries( + entries(PassStyleRankAndCover).map(([i, v]) => [v[0], Number(i)]), +); +setPrototypeOf(PassStyleRank, null); +harden(PassStyleRank); + +/** + * Associate with each passStyle a RankCover that may be an overestimate, + * and whose results therefore need to be filtered down. For example, because + * there is not a smallest or biggest bigint, bound it by `NaN` (the last place + * number) and `''` (the empty string, which is the first place string). Thus, + * a range query using this range may include these values, which would then + * need to be filtered out. + * + * @param {PassStyle} passStyle + * @returns {RankCover} + */ +export const getPassStyleCover = passStyle => + PassStyleRankAndCover[PassStyleRank[passStyle]][1]; +harden(getPassStyleCover); + +/** + * @type {WeakMap>} + */ +const memoOfSorted = new WeakMap(); + +/** + * @type {WeakMap} + */ +const comparatorMirrorImages = new WeakMap(); + +export const recordParts = record => { + assertRecord(record); + // TODO Measure which is faster: a reverse sort by sorting and + // reversing, or by sorting with an inverse comparison function. + // If it makes a significant difference, use the faster one. + const names = ownKeys(record) + .sort() + .reverse(); + // @ts-expect-error It thinks name might be a symbol, which it doesn't like. + const vals = names.map(name => record[name]); + return harden([names, vals]); +}; +harden(recordParts); + +/** + * @param {RankCompare=} compareRemotables + * An option to create a comparator in which an internal order is + * assigned to remotables. This defaults to a comparator that + * always returns `0`, meaning that all remotables are tied + * for the same rank. + * @returns {RankComparatorKit} + */ +export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => { + /** @type {RankCompare} */ + const comparator = (left, right) => { + if (sameValueZero(left, right)) { + return 0; + } + const leftStyle = passStyleOf(left); + const rightStyle = passStyleOf(right); + if (leftStyle !== rightStyle) { + return comparator(PassStyleRank[leftStyle], PassStyleRank[rightStyle]); + } + switch (leftStyle) { + case 'remotable': { + return compareRemotables(left, right); + } + case 'undefined': + case 'null': + case 'error': + case 'promise': { + // For each of these passStyles, all members of that passStyle are tied + // for the same rank. + return 0; + } + case 'boolean': + case 'bigint': + case 'string': { + // Within each of these passStyles, the rank ordering agrees with + // JavaScript's relational operators `<` and `>`. + if (left < right) { + return -1; + } else { + assert(left > right); + return 1; + } + } + case 'symbol': { + return comparator( + nameForPassableSymbol(left), + nameForPassableSymbol(right), + ); + } + case 'number': { + // `NaN`'s rank is after all other numbers. + if (Number.isNaN(left)) { + assert(!Number.isNaN(right)); + return 1; + } else if (Number.isNaN(right)) { + return -1; + } + // The rank ordering of non-NaN numbers agrees with JavaScript's + // relational operators '<' and '>'. + if (left < right) { + return -1; + } else { + assert(left > right); + return 1; + } + } + case 'copyRecord': { + // Lexicographic by inverse sorted order of property names, then + // lexicographic by corresponding values in that same inverse + // order of their property names. Comparing names by themselves first, + // all records with the exact same set of property names sort next to + // each other in a rank-sort of copyRecords. + + // The copyRecord invariants enforced by passStyleOf ensure that + // all the property names are strings. We need the reverse sorted order + // of these names, which we then compare lexicographically. This ensures + // that if the names of record X are a subset of the names of record Y, + // then record X will have an earlier rank and sort to the left of Y. + const [leftNames, leftValues] = recordParts(left); + const [rightNames, rightValues] = recordParts(right); + + const result = comparator(leftNames, rightNames); + if (result !== 0) { + return result; + } + return comparator(leftValues, rightValues); + } + case 'copyArray': { + // Lexicographic + const len = Math.min(left.length, right.length); + for (let i = 0; i < len; i += 1) { + const result = comparator(left[i], right[i]); + if (result !== 0) { + return result; + } + } + // If all matching elements were tied, then according to their lengths. + // If array X is a prefix of array Y, then X has an earlier rank than Y. + return comparator(left.length, right.length); + } + case 'tagged': { + // Lexicographic by `[Symbol.toStringTag]` then `.payload`. + const labelComp = comparator(getTag(left), getTag(right)); + if (labelComp !== 0) { + return labelComp; + } + return comparator(left.payload, right.payload); + } + default: { + assert.fail(X`Unrecognized passStyle: ${q(leftStyle)}`); + } + } + }; + + /** @type {RankCompare} */ + const antiComparator = (x, y) => comparator(y, x); + + memoOfSorted.set(comparator, new WeakSet()); + memoOfSorted.set(antiComparator, new WeakSet()); + comparatorMirrorImages.set(comparator, antiComparator); + comparatorMirrorImages.set(antiComparator, comparator); + + return harden({ comparator, antiComparator }); +}; +/** + * @param {RankCompare} comparator + * @returns {RankCompare=} + */ +export const comparatorMirrorImage = comparator => + comparatorMirrorImages.get(comparator); + +/** + * @param {Passable[]} passables + * @param {RankCompare} compare + * @returns {boolean} + */ +export const isRankSorted = (passables, compare) => { + const subMemoOfSorted = memoOfSorted.get(compare); + assert(subMemoOfSorted !== undefined); + if (subMemoOfSorted.has(passables)) { + return true; + } + assert(passStyleOf(passables) === 'copyArray'); + for (let i = 1; i < passables.length; i += 1) { + if (compare(passables[i - 1], passables[i]) >= 1) { + return false; + } + } + subMemoOfSorted.add(passables); + return true; +}; +harden(isRankSorted); + +/** + * @param {Passable[]} sorted + * @param {RankCompare} compare + */ +export const assertRankSorted = (sorted, compare) => + assert( + isRankSorted(sorted, compare), + // TODO assert on bug could lead to infinite recursion. Fix. + // eslint-disable-next-line no-use-before-define + X`Must be rank sorted: ${sorted} vs ${sortByRank(sorted, compare)}`, + ); +harden(assertRankSorted); + +/** + * TODO SECURITY BUG: https://github.com/Agoric/agoric-sdk/issues/4260 + * sortByRank currently uses `Array.prototype.sort` directly, and + * so only works correctly when given a `compare` function that considers + * `undefined` strictly bigger (`>`) than everything else. This is + * because `Array.prototype.sort` bizarrely moves all `undefined`s to + * the end of the array regardless, without consulting the `compare` + * function. This is a genuine bug for us NOW because sometimes we sort + * in reverse order by passing a reversed rank comparison function. + * + * @param {Iterable} passables + * @param {RankCompare} compare + * @returns {Passable[]} + */ +export const sortByRank = (passables, compare) => { + if (Array.isArray(passables)) { + harden(passables); + // Calling isRankSorted gives it a chance to get memoized for + // this `compare` function even if it was already memoized for a different + // `compare` function. + if (isRankSorted(passables, compare)) { + return passables; + } + } + const unsorted = [...passables]; + unsorted.forEach(harden); + const sorted = harden(unsorted.sort(compare)); + const subMemoOfSorted = memoOfSorted.get(compare); + assert(subMemoOfSorted !== undefined); + subMemoOfSorted.add(sorted); + return sorted; +}; +harden(sortByRank); + +/** + * See + * https://en.wikipedia.org/wiki/Binary_search_algorithm#Procedure_for_finding_the_leftmost_element + * + * @param {Passable[]} sorted + * @param {RankCompare} compare + * @param {Passable} key + * @param {("leftMost" | "rightMost")=} bias + * @returns {number} + */ +const rankSearch = (sorted, compare, key, bias = 'leftMost') => { + assertRankSorted(sorted, compare); + let left = 0; + let right = sorted.length; + while (left < right) { + const m = Math.floor((left + right) / 2); + const comp = compare(sorted[m], key); + if (comp <= -1 || (comp === 0 && bias === 'rightMost')) { + left = m + 1; + } else { + assert(comp >= 1 || (comp === 0 && bias === 'leftMost')); + right = m; + } + } + return bias === 'leftMost' ? left : right - 1; +}; + +/** + * @param {Passable[]} sorted + * @param {RankCompare} compare + * @param {RankCover} rankCover + * @returns {IndexCover} + */ +export const getIndexCover = (sorted, compare, [leftKey, rightKey]) => { + assertRankSorted(sorted, compare); + const leftIndex = rankSearch(sorted, compare, leftKey, 'leftMost'); + const rightIndex = rankSearch(sorted, compare, rightKey, 'rightMost'); + return [leftIndex, rightIndex]; +}; +harden(getIndexCover); + +/** @type {RankCover} */ +export const FullRankCover = harden(['', '{']); + +/** + * @param {Passable[]} sorted + * @param {IndexCover} indexCover + * @returns {Iterable<[number, Passable]>} + */ +export const coveredEntries = (sorted, [leftIndex, rightIndex]) => { + /** @type {Iterable<[number, Passable]>} */ + const iterable = harden({ + [Symbol.iterator]: () => { + let i = leftIndex; + return harden({ + next: () => { + if (i <= rightIndex) { + const element = sorted[i]; + i += 1; + return harden({ value: [i, element], done: false }); + } else { + return harden({ value: undefined, done: true }); + } + }, + }); + }, + }); + return iterable; +}; +harden(coveredEntries); + +/** + * @param {RankCompare} compare + * @param {Passable} a + * @param {Passable} b + * @returns {Passable} + */ +const maxRank = (compare, a, b) => (compare(a, b) >= 0 ? a : b); + +/** + * @param {RankCompare} compare + * @param {Passable} a + * @param {Passable} b + * @returns {Passable} + */ +const minRank = (compare, a, b) => (compare(a, b) <= 0 ? a : b); + +/** + * @param {RankCompare} compare + * @param {RankCover[]} covers + * @returns {RankCover} + */ +export const unionRankCovers = (compare, covers) => { + /** + * @param {RankCover} a + * @param {RankCover} b + * @returns {RankCover} + */ + const unionRankCoverPair = ([leftA, rightA], [leftB, rightB]) => [ + minRank(compare, leftA, leftB), + maxRank(compare, rightA, rightB), + ]; + return covers.reduce(unionRankCoverPair, ['{', '']); +}; +harden(unionRankCovers); + +/** + * @param {RankCompare} compare + * @param {RankCover[]} covers + * @returns {RankCover} + */ +export const intersectRankCovers = (compare, covers) => { + /** + * @param {RankCover} a + * @param {RankCover} b + * @returns {RankCover} + */ + const intersectRankCoverPair = ([leftA, rightA], [leftB, rightB]) => [ + maxRank(compare, leftA, leftB), + minRank(compare, rightA, rightB), + ]; + return covers.reduce(intersectRankCoverPair, ['', '{']); +}; + +export const { + comparator: compareRank, + antiComparator: compareAntiRank, +} = makeComparatorKit(); + +/** + * Create a comparator kit in which remotables are fully ordered + * by the order in which they are first seen by *this* comparator kit. + * BEWARE: This is observable mutable state, so such a comparator kit + * should never be shared among subsystems that should not be able + * to communicate. + * + * Note that this order does not meet the requirements for store + * ordering, since it has no memory of deleted keys. + * + * These full order comparator kit is strictly more precise that the + * rank order comparator kits above. As a result, any array which is + * sorted by such a full order will pass the isRankSorted test with + * a corresponding rank order. + * + * An array which is sorted by a *fresh* full order comparator, i.e., + * one that has not yet seen any remotables, will of course remain + * sorted by according to *that* full order comparator. An array *of + * scalars* sorted by a fresh full order will remain sorted even + * according to a new fresh full order comparator, since it will see + * the remotables in the same order again. Unfortunately, this is + * not true of arrays of passables in general. + * + * @param {boolean=} longLived + * @returns {FullComparatorKit} + */ +export const makeFullOrderComparatorKit = (longLived = false) => { + let numSeen = 0; + // When dynamically created with short lifetimes (the default) a WeakMap + // would perform poorly, and the leak created by a Map only lasts as long + // as the Map. + const MapConstructor = longLived ? WeakMap : Map; + const seen = new MapConstructor(); + const tag = r => { + if (seen.has(r)) { + return seen.get(r); + } + numSeen += 1; + seen.set(r, numSeen); + return numSeen; + }; + const compareRemotables = (x, y) => compareRank(tag(x), tag(y)); + return makeComparatorKit(compareRemotables); +}; +harden(makeFullOrderComparatorKit); diff --git a/packages/marshal/test/test-encodePassable.js b/packages/marshal/test/test-encodePassable.js new file mode 100644 index 0000000000..73c1480655 --- /dev/null +++ b/packages/marshal/test/test-encodePassable.js @@ -0,0 +1,129 @@ +// @ts-nocheck +/* eslint-disable no-bitwise */ + +import { test } from './prepare-test-env-ava.js'; +// eslint-disable-next-line import/order, import/no-extraneous-dependencies +import { fc } from '@fast-check/ava'; + +import { + makeEncodePassable, + makeDecodePassable, +} from '../src/encodePassable.js'; +import { compareRank, makeComparatorKit } from '../src/rankOrder.js'; +import { sample } from './test-rankOrder.js'; + +const { details: X } = assert; + +const r2e = new Map(); +const e2r = []; + +const encodeRemotable = r => { + if (r2e.has(r)) { + return r2e.get(r); + } + const result = `r${e2r.length}`; + r2e.set(r, result); + e2r.push(r); + return result; +}; + +const decodeRemotable = e => { + assert(e.startsWith('r'), X`unexpected encoding ${e}`); + const i = Number(BigInt(e.substring(1))); + assert(i >= 0 && i < e2r.length); + return e2r[i]; +}; + +const compareRemotables = (x, y) => + compareRank(encodeRemotable(x), encodeRemotable(y)); + +const encodeKey = makeEncodePassable({ encodeRemotable }); + +const decodeKey = makeDecodePassable({ decodeRemotable }); + +const { comparator: compareFull } = makeComparatorKit(compareRemotables); + +const asNumber = new Float64Array(1); +const asBits = new BigUint64Array(asNumber.buffer); +const getNaN = (hexEncoding = '0008000000000000') => { + let bits = BigInt(`0x${hexEncoding}`); + bits |= 0x7ff0000000000000n; + if (!(bits & 0x0001111111111111n)) { + bits |= 0x0008000000000000n; + } + asBits[0] = bits; + return asNumber[0]; +}; + +const NegativeNaN = getNaN('ffffffffffffffff'); + +/** @type {[Key, string][]} */ +const goldenPairs = harden([ + [1, 'fbff0000000000000'], + [-1, 'f400fffffffffffff'], + [NaN, 'ffff8000000000000'], + [NegativeNaN, 'ffff8000000000000'], + [0, 'f8000000000000000'], + [Infinity, 'ffff0000000000000'], + [-Infinity, 'f000fffffffffffff'], + [-1234567890n, 'n#90:8765432110'], + [-123456789n, 'n1:876543211'], + [-1000n, 'n6:9000'], + [-999n, 'n7:001'], + [-1n, 'n9:9'], + [-0n, 'p1:0'], + [37n, 'p2:37'], + [123456789n, 'p9:123456789'], + [1234567890n, 'p~10:1234567890'], + [934857932847598725662n, 'p~21:934857932847598725662'], +]); + +test('golden round trips', t => { + for (const [k, e] of goldenPairs) { + t.is(encodeKey(k), e, 'does k encode as expected'); + t.is(decodeKey(e), k, 'does the key round trip through the encoding'); + } + // Not round trips + t.is(encodeKey(-0), 'f8000000000000000'); + t.is(decodeKey('f0000000000000000'), NaN); +}); + +const orderInvariants = (t, x, y) => { + const rankComp = compareRank(x, y); + const fullComp = compareFull(x, y); + if (rankComp !== 0) { + t.is(rankComp, fullComp); + } + if (fullComp === 0) { + t.is(rankComp, 0); + } else { + t.assert(rankComp === 0 || rankComp === fullComp); + } +}; + +test('order invariants', t => { + for (let i = 0; i < sample.length; i += 1) { + for (let j = i; j < sample.length; j += 1) { + orderInvariants(t, sample[i], sample[j]); + } + } +}); + +test('BigInt values round-trip', async t => { + await fc.assert( + fc.property(fc.bigInt(), n => { + const rt = decodeKey(encodeKey(n)); + return t.is(rt, n); + }), + ); +}); + +test('BigInt encoding comparison corresponds with numeric comparison', async t => { + await fc.assert( + fc.property(fc.bigInt(), fc.bigInt(), (a, b) => { + const ea = encodeKey(a); + const eb = encodeKey(b); + return t.is(a < b, ea < eb) && t.is(a > b, ea > eb); + }), + ); +}); diff --git a/packages/marshal/test/test-rankOrder.js b/packages/marshal/test/test-rankOrder.js new file mode 100644 index 0000000000..8658ff4ba8 --- /dev/null +++ b/packages/marshal/test/test-rankOrder.js @@ -0,0 +1,377 @@ +// @ts-nocheck +import { test } from './prepare-test-env-ava.js'; +// eslint-disable-next-line import/order, import/no-extraneous-dependencies +import { fc } from '@fast-check/ava'; + +import { + FullRankCover, + compareRank, + isRankSorted, + sortByRank, + getIndexCover, + getPassStyleCover, + assertRankSorted, +} from '../src/rankOrder.js'; + +import { makeTagged } from '../src/makeTagged.js'; +import { Far } from '../src/make-far.js'; + +const { quote: q } = assert; + +/** + * The only elements with identity. Everything else should be equal + * by contents. + */ +const alice = Far('alice', {}); +const bob = Far('bob', {}); +const carol = Far('carol', {}); + +/** + * A factory for arbitrary passables + */ +const { passable } = fc.letrec(tie => { + return { + passable: tie('dag').map(x => harden(x)), + dag: fc.oneof( + { depthFactor: 0.5, withCrossShrink: true }, + // a tagged value whose payload is an array of [key, leaf] pairs + // where each key is unique within the payload + // XXX can the payload be generalized further? + fc + .record({ + type: fc.constantFrom('copyMap', 'copySet', 'nonsense'), + payload: fc + .uniqueArray(fc.fullUnicodeString(), { maxLength: 3 }) + .chain(k => { + return fc.tuple(fc.constant(k), tie('leaf')); + }), + }) + .map(({ type, payload }) => makeTagged(type, payload)), + fc.array(tie('dag'), { maxLength: 3 }), + fc.dictionary( + fc.fullUnicodeString().filter(s => s !== 'then'), + tie('dag'), + { maxKeys: 3 }, + ), + tie('dag').map(v => Promise.resolve(v)), + tie('leaf'), + ), + leaf: fc.oneof( + fc.record({}), + fc.fullUnicodeString(), + fc.fullUnicodeString().map(s => Symbol.for(s)), + fc.fullUnicodeString().map(s => new Error(s)), + // primordial symbols and registered lookalikes + fc.constantFrom( + ...Object.getOwnPropertyNames(Symbol).flatMap(k => { + const v = Symbol[k]; + if (typeof v !== 'symbol') return []; + return [v, Symbol.for(k), Symbol.for(`@@${k}`)]; + }), + ), + fc.bigInt(), + fc.integer(), + fc.constantFrom(-0, NaN, Infinity, -Infinity), + fc.constantFrom(null, undefined, false, true), + fc.constantFrom(alice, bob, carol), + // unresolved promise + fc.constant(new Promise(() => {})), + ), + }; +}); + +test('compareRank is reflexive', async t => { + await fc.assert( + fc.property(passable, x => { + return t.is(compareRank(x, x), 0); + }), + ); +}); + +test('compareRank totally orders ranks', async t => { + await fc.assert( + fc.property(passable, passable, (a, b) => { + const ab = compareRank(a, b); + const ba = compareRank(b, a); + if (ab === 0) { + return t.is(ba, 0); + } + return ( + t.true(Math.abs(ab) > 0) && + t.true(Math.abs(ba) > 0) && + t.is(Math.sign(ba), -Math.sign(ab)) + ); + }), + ); +}); + +// TODO Had to remove key-level cases from the test-encodePassable.js as +// migrated to endo. As a result, some of the tests here are broken. +// Fix. +test.skip('compareRank is transitive', async t => { + await fc.assert( + fc.property( + // operate on a set of three passables covering at least two ranks + fc + .uniqueArray(passable, { minLength: 3, maxLength: 3 }) + .filter( + ([a, b, c]) => compareRank(a, b) !== 0 || compareRank(a, c) !== 0, + ), + triple => { + const sorted = harden(triple.sort(compareRank)); + assertRankSorted(sorted, compareRank); + const [a, b, c] = sorted; + const failures = []; + let result; + let resultOk; + + result = compareRank(a, b); + resultOk = t.true(result <= 0, 'a <= b'); + if (!resultOk) { + failures.push(`Expected <= 0: ${result} from ${q(a)} vs. ${q(b)}`); + } + result = compareRank(a, c); + resultOk = t.true(result <= 0, 'a <= c'); + if (!resultOk) { + failures.push(`Expected <= 0: ${result} from ${q(a)} vs. ${q(c)}`); + } + result = compareRank(b, c); + resultOk = t.true(result <= 0, 'b <= c'); + if (!resultOk) { + failures.push(`Expected <= 0: ${result} from ${q(b)} vs. ${q(c)}`); + } + result = compareRank(c, b); + resultOk = t.true(result >= 0, 'c >= b'); + if (!resultOk) { + failures.push(`Expected >= 0: ${result} from ${q(c)} vs. ${q(b)}`); + } + result = compareRank(c, a); + resultOk = t.true(result >= 0, 'c >= a'); + if (!resultOk) { + failures.push(`Expected >= 0: ${result} from ${q(c)} vs. ${q(a)}`); + } + result = compareRank(b, a); + resultOk = t.true(result >= 0, 'b >= a'); + if (!resultOk) { + failures.push(`Expected >= 0: ${result} from ${q(b)} vs. ${q(a)}`); + } + + return t.deepEqual(failures, []); + }, + ), + ); +}); + +/** + * An unordered copyArray of some passables + */ +export const sample = harden([ + makeTagged('copySet', [ + ['b', 3], + ['a', 4], + ]), + 'foo', + 3n, + 'barr', + undefined, + [5, { foo: 4 }], + 2, + null, + [5, { foo: 4, bar: null }], + bob, + 0, + makeTagged('copySet', [ + ['a', 4], + ['b', 3], + ]), + NaN, + true, + undefined, + -Infinity, + [5], + alice, + [], + Symbol.for('foo'), + new Error('not erroneous'), + Symbol.for('@@foo'), + [5, { bar: 5 }], + Symbol.for(''), + false, + carol, + -0, + {}, + [5, undefined], + -3, + makeTagged('copyMap', [ + ['a', 4], + ['b', 3], + ]), + true, + 'bar', + [5, null], + new Promise(() => {}), // forever unresolved + makeTagged('nonsense', [ + ['a', 4], + ['b', 3], + ]), + Infinity, + Symbol.isConcatSpreadable, + [5, { foo: 4, bar: undefined }], + Promise.resolve('fulfillment'), + [5, { foo: 4 }], +]); + +const rejectedP = Promise.reject(new Error('broken')); +rejectedP.catch(() => {}); // Suppress unhandled rejection warning/error + +/** + * The correctly stable rank sorting of `sample` + */ +const sortedSample = harden([ + // All errors are tied. + new Error('different'), + + {}, + + // Lexicographic tagged: tag then payload + makeTagged('copyMap', [ + ['a', 4], + ['b', 3], + ]), + makeTagged('copySet', [ + ['a', 4], + ['b', 3], + ]), + // Doesn't care if a valid copySet + makeTagged('copySet', [ + ['b', 3], + ['a', 4], + ]), + // Doesn't care if a recognized tagged tag + makeTagged('nonsense', [ + ['a', 4], + ['b', 3], + ]), + + // All promises are tied. + rejectedP, + rejectedP, + + // Lexicographic arrays. Shorter beats longer. + // Lexicographic records by reverse sorted property name, then by values + // in that order. + [], + [5], + [5, { bar: 5 }], + [5, { foo: 4 }], + [5, { foo: 4 }], + [5, { foo: 4, bar: null }], + [5, { foo: 4, bar: undefined }], + [5, null], + [5, undefined], + + false, + true, + true, + + // -0 is equivalent enough to 0. NaN after all numbers. + -Infinity, + -3, + -0, + 0, + 2, + Infinity, + NaN, + + 3n, + + // All remotables are tied for the same rank and the sort is stable, + // so their relative order is preserved + bob, + alice, + carol, + + // Lexicographic strings. Shorter beats longer. + // TODO Probe UTF-16 vs Unicode vs UTF-8 (Moddable) ordering. + 'bar', + 'barr', + 'foo', + + null, + Symbol.for(''), + Symbol.for('@@foo'), + Symbol.isConcatSpreadable, + Symbol.for('foo'), + + undefined, + undefined, +]); + +test('compare and sort by rank', t => { + assertRankSorted(sortedSample, compareRank); + t.false(isRankSorted(sample, compareRank)); + const sorted = sortByRank(sample, compareRank); + t.is( + compareRank(sorted, sortedSample), + 0, + `Not sorted as expected: ${q(sorted)}`, + ); +}); + +const rangeSample = harden([ + {}, // 0 -- prefix are earlier, so empty is earliest + { bar: null }, // 1 + { bar: undefined }, // 2 -- records with same names grouped together + { foo: 'x' }, // 3 -- name subsets before supersets + { bar: 'y', foo: 'x' }, // 5 + { bar: 'y', foo: 'x' }, // 6 + { bar: null, foo: 'x' }, // 4 + { bar: undefined, foo: 'x' }, // 7 + { bar: 'y', foo: 'y' }, // 8 -- reverse sort so foo: tested before bar: + + makeTagged('', null), // 9 + + ['a'], // 10 + ['a', 'b'], // 11 + ['a', 'x'], // 12 + ['y', 'x'], // 13 +]); + +/** @type {[RankCover, IndexCover][]} */ +const queries = harden([ + [ + [['c'], ['c']], + // first > last implies absent. + [12, 11], + ], + [ + [['a'], ['a', undefined]], + [9, 11], + ], + [ + [ + ['a', null], + ['a', undefined], + ], + [10, 11], + ], + [FullRankCover, [0, 13]], + [getPassStyleCover('string'), [0, -1]], + [getPassStyleCover('copyRecord'), [0, 8]], + [getPassStyleCover('copyArray'), [9, 13]], // cover includes non-array + [getPassStyleCover('remotable'), [14, 13]], +]); + +// XXX This test is skipped because of unresolved impedance mismatch between the +// older value-as-cover scheme and the newer string-encoded-key-as-cover scheme +// that we currently use. Whoever sorts that mismatch out (likely as part of +// adding composite key handling to the durable store implementation) will need +// to re-enable and (likely) update this test. +test.skip('range queries', t => { + t.assert(isRankSorted(rangeSample, compareRank)); + for (const [rankCover, indexRange] of queries) { + const range = getIndexCover(rangeSample, compareRank, rankCover); + t.is(range[0], indexRange[0]); + t.is(range[1], indexRange[1]); + } +}); diff --git a/yarn.lock b/yarn.lock index fcb6c0fd51..7406dd6d79 100644 --- a/yarn.lock +++ b/yarn.lock @@ -975,6 +975,13 @@ unique-filename "^1.1.1" which "^1.3.1" +"@fast-check/ava@^1.0.1": + version "1.0.1" + resolved "https://registry.yarnpkg.com/@fast-check/ava/-/ava-1.0.1.tgz#d116fd93cbfc59a3011bf74894bd9906cb7a2753" + integrity sha512-9BAgwNS4WRf/tEZAiDje1xgRpKYeytc6L9s+S0/nYzxLf2TfOBDJQUFQqVRgZRa1dj7+bickUOZTnPiNVr1rng== + dependencies: + fast-check "^3.0.0" + "@humanwhocodes/config-array@^0.5.0": version "0.5.0" resolved "https://registry.yarnpkg.com/@humanwhocodes/config-array/-/config-array-0.5.0.tgz#1407967d4c6eecd7388f83acf1eaf4d0c6e58ef9" @@ -5670,6 +5677,13 @@ falafel@^2.1.0: acorn "^7.1.1" isarray "^2.0.1" +fast-check@^3.0.0: + version "3.1.1" + resolved "https://registry.yarnpkg.com/fast-check/-/fast-check-3.1.1.tgz#72c5ae7022a4e86504762e773adfb8a5b0b01252" + integrity sha512-3vtXinVyuUKCKFKYcwXhGE6NtGWkqF8Yh3rvMZNzmwz8EPrgoc/v4pDdLHyLnCyCI5MZpZZkDEwFyXyEONOxpA== + dependencies: + pure-rand "^5.0.1" + fast-deep-equal@^3.1.1, fast-deep-equal@^3.1.3: version "3.1.3" resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz#3a7d56b559d6cbc3eb512325244e619a65c6c525" @@ -10418,6 +10432,11 @@ puppeteer@^1.13.0: rimraf "^2.6.1" ws "^6.1.0" +pure-rand@^5.0.1: + version "5.0.1" + resolved "https://registry.yarnpkg.com/pure-rand/-/pure-rand-5.0.1.tgz#97a287b4b4960b2a3448c0932bf28f2405cac51d" + integrity sha512-ksWccjmXOHU2gJBnH0cK1lSYdvSZ0zLoCMSz/nTGh6hDvCSgcRxDyIcOBD6KNxFz3xhMPm/T267Tbe2JRymKEQ== + purgecss@^2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/purgecss/-/purgecss-2.3.0.tgz#5327587abf5795e6541517af8b190a6fb5488bb3" From 19218906c5a8c3c0e22c3f55066af797ee0a49fb Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Fri, 4 Nov 2022 22:19:07 -0700 Subject: [PATCH 2/5] fix: review comments --- packages/marshal/src/encodePassable.js | 45 ++++++++++++++------------ packages/marshal/src/rankOrder.js | 14 ++++---- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/packages/marshal/src/encodePassable.js b/packages/marshal/src/encodePassable.js index 9993b21a03..fdd0dc1e13 100644 --- a/packages/marshal/src/encodePassable.js +++ b/packages/marshal/src/encodePassable.js @@ -69,7 +69,8 @@ const encodeBinary64 = n => { }; const decodeBinary64 = encoded => { - assert(encoded.startsWith('f'), X`Encoded number expected: ${encoded}`); + encoded.startsWith('f') || + assert.fail(X`Encoded number expected: ${encoded}`); let bits = BigInt(`0x${encoded.substring(1)}`); if (encoded[1] < '8') { bits ^= 0xffffffffffffffffn; @@ -78,7 +79,7 @@ const decodeBinary64 = encoded => { } asBits[0] = bits; const result = asNumber[0]; - assert(!is(result, -0), X`Unexpected negative zero: ${encoded}`); + !is(result, -0) || assert.fail(X`Unexpected negative zero: ${encoded}`); return result; }; @@ -126,17 +127,18 @@ const encodeBigInt = n => { }; const decodeBigInt = encoded => { - const typePrefix = encoded[0]; + const typePrefix = encoded.charAt(0); // faster than encoded[0] let rem = encoded.slice(1); typePrefix === 'p' || typePrefix === 'n' || assert.fail(X`Encoded bigint expected: ${encoded}`); const lDigits = rem.search(/[0-9]/) + 1; - assert(lDigits >= 1, X`Digit count expected: ${encoded}`); + lDigits >= 1 || assert.fail(X`Digit count expected: ${encoded}`); rem = rem.slice(lDigits - 1); - assert(rem.length >= lDigits, X`Complete digit count expected: ${encoded}`); + rem.length >= lDigits || + assert.fail(X`Complete digit count expected: ${encoded}`); const snDigits = rem.slice(0, lDigits); rem = rem.slice(lDigits); /^[0-9]+$/.test(snDigits) || @@ -148,7 +150,7 @@ const decodeBigInt = encoded => { nDigits = 10 ** lDigits - nDigits; } - assert(rem.startsWith(':'), X`Separator expected: ${encoded}`); + rem.startsWith(':') || assert.fail(X`Separator expected: ${encoded}`); rem = rem.slice(1); rem.length === nDigits || assert.fail(X`Fixed-length digit sequence expected: ${encoded}`); @@ -182,7 +184,7 @@ const encodeArray = (array, encodePassable) => { }; const decodeArray = (encoded, decodePassable) => { - assert(encoded.startsWith('['), X`Encoded array expected: ${encoded}`); + encoded.startsWith('[') || assert.fail(X`Encoded array expected: ${encoded}`); const elements = []; const elemChars = []; for (let i = 1; i < encoded.length; i += 1) { @@ -194,7 +196,8 @@ const decodeArray = (encoded, decodePassable) => { elements.push(element); } else if (c === '\u0001') { i += 1; - assert(i < encoded.length, X`unexpected end of encoding ${encoded}`); + i < encoded.length || + assert.fail(X`unexpected end of encoding ${encoded}`); const c2 = encoded[i]; c2 === '\u0000' || c2 === '\u0001' || @@ -204,7 +207,8 @@ const decodeArray = (encoded, decodePassable) => { elemChars.push(c); } } - assert(elemChars.length === 0, X`encoding terminated early: ${encoded}`); + elemChars.length === 0 || + assert.fail(X`encoding terminated early: ${encoded}`); return harden(elements); }; @@ -216,15 +220,15 @@ const encodeRecord = (record, encodePassable) => { const decodeRecord = (encoded, decodePassable) => { assert(encoded.startsWith('(')); const keysvals = decodeArray(encoded.substring(1), decodePassable); - assert(keysvals.length === 2, X`expected keys,values pair: ${encoded}`); + keysvals.length === 2 || + assert.fail(X`expected keys,values pair: ${encoded}`); const [keys, vals] = keysvals; - assert( - passStyleOf(keys) === 'copyArray' && - passStyleOf(vals) === 'copyArray' && - keys.length === vals.length && - keys.every(key => typeof key === 'string'), - X`not a valid record encoding: ${encoded}`, - ); + + (passStyleOf(keys) === 'copyArray' && + passStyleOf(vals) === 'copyArray' && + keys.length === vals.length && + keys.every(key => typeof key === 'string')) || + assert.fail(X`not a valid record encoding: ${encoded}`); const entries = keys.map((key, i) => [key, vals[i]]); const record = harden(fromEntries(entries)); assertRecord(record, 'decoded record'); @@ -237,7 +241,8 @@ const encodeTagged = (tagged, encodePassable) => const decodeTagged = (encoded, decodePassable) => { assert(encoded.startsWith(':')); const tagpayload = decodeArray(encoded.substring(1), decodePassable); - assert(tagpayload.length === 2, X`expected tag,payload pair: ${encoded}`); + tagpayload.length === 2 || + assert.fail(X`expected tag,payload pair: ${encoded}`); const [tag, payload] = tagpayload; passStyleOf(tag) === 'string' || assert.fail(X`not a valid tagged encoding: ${encoded}`); @@ -380,7 +385,7 @@ export const makeDecodePassable = ({ decodeError = (err, _) => assert.fail(X`error unexpected: ${err}`), } = {}) => { const decodePassable = encoded => { - switch (encoded[0]) { + switch (encoded.charAt(0)) { case 'v': { return null; } @@ -430,5 +435,5 @@ export const makeDecodePassable = ({ }; harden(makeDecodePassable); -export const isEncodedRemotable = encoded => encoded[0] === 'r'; +export const isEncodedRemotable = encoded => encoded.charAt(0) === 'r'; harden(isEncodedRemotable); diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js index 6b462c569f..3b6071a460 100644 --- a/packages/marshal/src/rankOrder.js +++ b/packages/marshal/src/rankOrder.js @@ -21,7 +21,7 @@ const { ownKeys } = Reflect; /** * @callback RankCompare * Returns `-1`, `0`, or `1` depending on whether the rank of `left` - * is before, tied-with, or after the rank of `right`. + * is respectively before, tied-with, or after the rank of `right`. * * This comparison function is valid as argument to * `Array.prototype.sort`. This is sometimes described as a "total order" @@ -157,9 +157,11 @@ const comparatorMirrorImages = new WeakMap(); export const recordParts = record => { assertRecord(record); - // TODO Measure which is faster: a reverse sort by sorting and - // reversing, or by sorting with an inverse comparison function. - // If it makes a significant difference, use the faster one. + // https://github.com/endojs/endo/pull/1260#discussion_r1003657244 + // compares two ways of reverse sorting, and shows that this way + // is currently faster on Moddable XS while the other way, + // `.sort(reverseComparator)`, is faster on v8. We currently care more about + // XS performance, so we reverse sort this way. const names = ownKeys(record) .sort() .reverse(); @@ -327,8 +329,8 @@ harden(isRankSorted); * @param {RankCompare} compare */ export const assertRankSorted = (sorted, compare) => - assert( - isRankSorted(sorted, compare), + isRankSorted(sorted, compare) || + assert.fail( // TODO assert on bug could lead to infinite recursion. Fix. // eslint-disable-next-line no-use-before-define X`Must be rank sorted: ${sorted} vs ${sortByRank(sorted, compare)}`, From 8ac6c3dd10277824a3127670bd2c50cc5e4c5637 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Fri, 4 Nov 2022 23:10:47 -0700 Subject: [PATCH 3/5] fix: move rankCover to encodePassable --- packages/marshal/src/encodePassable.js | 77 +++++++++++++++++++++++-- packages/marshal/src/rankOrder.js | 70 +--------------------- packages/marshal/src/types.js | 7 +++ packages/marshal/test/test-rankOrder.js | 2 +- 4 files changed, 84 insertions(+), 72 deletions(-) diff --git a/packages/marshal/src/encodePassable.js b/packages/marshal/src/encodePassable.js index fdd0dc1e13..4ee9cfb85e 100644 --- a/packages/marshal/src/encodePassable.js +++ b/packages/marshal/src/encodePassable.js @@ -8,14 +8,37 @@ import { nameForPassableSymbol, passableSymbolForName, } from './helpers/symbol.js'; -import { recordParts } from './rankOrder.js'; import { ErrorHelper } from './helpers/error.js'; +/** @typedef {import('./types.js').PassStyle} PassStyle */ /** @typedef {import('./types.js').Passable} Passable */ /** @typedef {import('./types.js').Remotable} Remotable */ +/** @template T @typedef {import('./types.js').CopyRecord} CopyRecord */ +/** @typedef {import('./types.js').RankCover} RankCover */ const { details: X, quote: q } = assert; -const { is, fromEntries } = Object; +const { fromEntries, setPrototypeOf, is } = Object; +const { ownKeys } = Reflect; + +/** + * @template T + * @param {CopyRecord} record + * @returns {[string[], T[]]} + */ +export const recordParts = record => { + assertRecord(record); + // https://github.com/endojs/endo/pull/1260#discussion_r1003657244 + // compares two ways of reverse sorting, and shows that this way + // is currently faster on Moddable XS while the other way, + // `.sort(reverseComparator)`, is faster on v8. We currently care more about + // XS performance, so we reverse sort this way. + const names = /** @type {string[]} */ (ownKeys(record) + .sort() + .reverse()); + const vals = names.map(name => record[name]); + return harden([names, vals]); +}; +harden(recordParts); export const zeroPad = (n, size) => { const nStr = `${n}`; @@ -229,8 +252,8 @@ const decodeRecord = (encoded, decodePassable) => { keys.length === vals.length && keys.every(key => typeof key === 'string')) || assert.fail(X`not a valid record encoding: ${encoded}`); - const entries = keys.map((key, i) => [key, vals[i]]); - const record = harden(fromEntries(entries)); + const mapEntries = keys.map((key, i) => [key, vals[i]]); + const record = harden(fromEntries(mapEntries)); assertRecord(record, 'decoded record'); return record; }; @@ -437,3 +460,49 @@ harden(makeDecodePassable); export const isEncodedRemotable = encoded => encoded.charAt(0) === 'r'; harden(isEncodedRemotable); + +// ///////////////////////////////////////////////////////////////////////////// + +/** + * @type {[PassStyle, RankCover][]} + */ +const PassStyleRankAndCover = harden([ + /* ! */ ['error', ['!', '!~']], + /* ( */ ['copyRecord', ['(', '(~']], + /* : */ ['tagged', [':', ':~']], + /* ? */ ['promise', ['?', '?~']], + /* [ */ ['copyArray', ['[', '[~']], + /* b */ ['boolean', ['b', 'b~']], + /* f */ ['number', ['f', 'f~']], + /* np */ ['bigint', ['n', 'p~']], + /* r */ ['remotable', ['r', 'r~']], + /* s */ ['string', ['s', 't']], + /* v */ ['null', ['v', 'v~']], + /* y */ ['symbol', ['y', 'z']], + /* z */ ['undefined', ['z', '{']], + /* | remotable->ordinal mapping prefix: This is not used in covers but it is + reserved from the same set of strings. Note that the prefix is > any + prefix used by any cover so that ordinal mapping keys are always outside + the range of valid collection entry keys. */ +]); + +export const PassStyleRank = fromEntries( + PassStyleRankAndCover.map(([passStyle, _range], i) => [passStyle, i]), +); +setPrototypeOf(PassStyleRank, null); +harden(PassStyleRank); + +/** + * Associate with each passStyle a RankCover that may be an overestimate, + * and whose results therefore need to be filtered down. For example, because + * there is not a smallest or biggest bigint, bound it by `NaN` (the last place + * number) and `''` (the empty string, which is the first place string). Thus, + * a range query using this range may include these values, which would then + * need to be filtered out. + * + * @param {PassStyle} passStyle + * @returns {RankCover} + */ +export const getPassStyleCover = passStyle => + PassStyleRankAndCover[PassStyleRank[passStyle]][1]; +harden(getPassStyleCover); diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js index 3b6071a460..4f2cdede95 100644 --- a/packages/marshal/src/rankOrder.js +++ b/packages/marshal/src/rankOrder.js @@ -1,15 +1,15 @@ // @ts-check import { getTag } from './helpers/passStyle-helpers.js'; import { passStyleOf } from './passStyleOf.js'; -import { assertRecord } from './typeGuards.js'; import { nameForPassableSymbol } from './helpers/symbol.js'; +import { PassStyleRank, recordParts } from './encodePassable.js'; /** @typedef {import('./types.js').Passable} Passable */ /** @typedef {import('./types.js').PassStyle} PassStyle */ +/** @typedef {import('./types.js').RankCover} RankCover */ const { details: X, quote: q } = assert; -const { fromEntries, entries, setPrototypeOf, is } = Object; -const { ownKeys } = Reflect; +const { is } = Object; /** * @typedef {-1 | 0 | 1} RankComparison @@ -78,10 +78,6 @@ const { ownKeys } = Reflect; * @property {FullCompare} antiComparator */ -/** - * @typedef {[string, string]} RankCover - */ - /** * @typedef {[number, number]} IndexCover */ @@ -101,50 +97,6 @@ const { ownKeys } = Reflect; */ const sameValueZero = (x, y) => x === y || is(x, y); -/** - * @type {[PassStyle, RankCover][]} - */ -const PassStyleRankAndCover = harden([ - /* ! */ ['error', ['!', '!~']], - /* ( */ ['copyRecord', ['(', '(~']], - /* : */ ['tagged', [':', ':~']], - /* ? */ ['promise', ['?', '?~']], - /* [ */ ['copyArray', ['[', '[~']], - /* b */ ['boolean', ['b', 'b~']], - /* f */ ['number', ['f', 'f~']], - /* np */ ['bigint', ['n', 'p~']], - /* r */ ['remotable', ['r', 'r~']], - /* s */ ['string', ['s', 't']], - /* v */ ['null', ['v', 'v~']], - /* y */ ['symbol', ['y', 'z']], - /* z */ ['undefined', ['z', '{']], - /* | remotable->ordinal mapping prefix: This is not used in covers but it is - reserved from the same set of strings. Note that the prefix is > any - prefix used by any cover so that ordinal mapping keys are always outside - the range of valid collection entry keys. */ -]); - -const PassStyleRank = fromEntries( - entries(PassStyleRankAndCover).map(([i, v]) => [v[0], Number(i)]), -); -setPrototypeOf(PassStyleRank, null); -harden(PassStyleRank); - -/** - * Associate with each passStyle a RankCover that may be an overestimate, - * and whose results therefore need to be filtered down. For example, because - * there is not a smallest or biggest bigint, bound it by `NaN` (the last place - * number) and `''` (the empty string, which is the first place string). Thus, - * a range query using this range may include these values, which would then - * need to be filtered out. - * - * @param {PassStyle} passStyle - * @returns {RankCover} - */ -export const getPassStyleCover = passStyle => - PassStyleRankAndCover[PassStyleRank[passStyle]][1]; -harden(getPassStyleCover); - /** * @type {WeakMap>} */ @@ -155,22 +107,6 @@ const memoOfSorted = new WeakMap(); */ const comparatorMirrorImages = new WeakMap(); -export const recordParts = record => { - assertRecord(record); - // https://github.com/endojs/endo/pull/1260#discussion_r1003657244 - // compares two ways of reverse sorting, and shows that this way - // is currently faster on Moddable XS while the other way, - // `.sort(reverseComparator)`, is faster on v8. We currently care more about - // XS performance, so we reverse sort this way. - const names = ownKeys(record) - .sort() - .reverse(); - // @ts-expect-error It thinks name might be a symbol, which it doesn't like. - const vals = names.map(name => record[name]); - return harden([names, vals]); -}; -harden(recordParts); - /** * @param {RankCompare=} compareRemotables * An option to create a comparator in which an internal order is diff --git a/packages/marshal/src/types.js b/packages/marshal/src/types.js index f4cd33b54e..6a9d1c80d5 100644 --- a/packages/marshal/src/types.js +++ b/packages/marshal/src/types.js @@ -263,3 +263,10 @@ export {}; * @param {Details=} details * @returns {boolean} */ + +/** + * @typedef {[string, string]} RankCover + * RankCover represents the inclusive lower bound and *inclusive* upper bound + * of a string-comparison range that covers all possible encodings for + * a set of values. + */ diff --git a/packages/marshal/test/test-rankOrder.js b/packages/marshal/test/test-rankOrder.js index 8658ff4ba8..de15076ca2 100644 --- a/packages/marshal/test/test-rankOrder.js +++ b/packages/marshal/test/test-rankOrder.js @@ -9,9 +9,9 @@ import { isRankSorted, sortByRank, getIndexCover, - getPassStyleCover, assertRankSorted, } from '../src/rankOrder.js'; +import { getPassStyleCover } from '../src/encodePassable.js'; import { makeTagged } from '../src/makeTagged.js'; import { Far } from '../src/make-far.js'; From 8378962e3034c9a2c3e5ac4a7c6c87afa9645b8a Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sun, 6 Nov 2022 21:25:53 -0800 Subject: [PATCH 4/5] fix: review refactor suggestion --- packages/marshal/src/encodePassable.js | 70 ++++++++++--------------- packages/marshal/src/rankOrder.js | 51 ++++++++++++++++-- packages/marshal/test/test-rankOrder.js | 2 +- 3 files changed, 78 insertions(+), 45 deletions(-) diff --git a/packages/marshal/src/encodePassable.js b/packages/marshal/src/encodePassable.js index 4ee9cfb85e..4b6d1b16a6 100644 --- a/packages/marshal/src/encodePassable.js +++ b/packages/marshal/src/encodePassable.js @@ -17,7 +17,7 @@ import { ErrorHelper } from './helpers/error.js'; /** @typedef {import('./types.js').RankCover} RankCover */ const { details: X, quote: q } = assert; -const { fromEntries, setPrototypeOf, is } = Object; +const { fromEntries, is } = Object; const { ownKeys } = Reflect; /** @@ -464,45 +464,33 @@ harden(isEncodedRemotable); // ///////////////////////////////////////////////////////////////////////////// /** - * @type {[PassStyle, RankCover][]} - */ -const PassStyleRankAndCover = harden([ - /* ! */ ['error', ['!', '!~']], - /* ( */ ['copyRecord', ['(', '(~']], - /* : */ ['tagged', [':', ':~']], - /* ? */ ['promise', ['?', '?~']], - /* [ */ ['copyArray', ['[', '[~']], - /* b */ ['boolean', ['b', 'b~']], - /* f */ ['number', ['f', 'f~']], - /* np */ ['bigint', ['n', 'p~']], - /* r */ ['remotable', ['r', 'r~']], - /* s */ ['string', ['s', 't']], - /* v */ ['null', ['v', 'v~']], - /* y */ ['symbol', ['y', 'z']], - /* z */ ['undefined', ['z', '{']], - /* | remotable->ordinal mapping prefix: This is not used in covers but it is - reserved from the same set of strings. Note that the prefix is > any - prefix used by any cover so that ordinal mapping keys are always outside - the range of valid collection entry keys. */ -]); - -export const PassStyleRank = fromEntries( - PassStyleRankAndCover.map(([passStyle, _range], i) => [passStyle, i]), -); -setPrototypeOf(PassStyleRank, null); -harden(PassStyleRank); - -/** - * Associate with each passStyle a RankCover that may be an overestimate, - * and whose results therefore need to be filtered down. For example, because - * there is not a smallest or biggest bigint, bound it by `NaN` (the last place - * number) and `''` (the empty string, which is the first place string). Thus, - * a range query using this range may include these values, which would then - * need to be filtered out. + * @type {Record} + * The single prefix characters to be used for each PassStyle category. + * `bigint` is a two character string because each of those characters + * individually is a valid bigint prefix. `n` for "negative" and `p` for + * "positive". The ordering of these prefixes is the same as the + * rankOrdering of their respective PassStyles. This table is imported by + * randOrder.js for this purpose. * - * @param {PassStyle} passStyle - * @returns {RankCover} + * In addition, `|` is the remotable->ordinal mapping prefix: + * This is not used in covers but it is + * reserved from the same set of strings. Note that the prefix is > any + * prefix used by any cover so that ordinal mapping keys are always outside + * the range of valid collection entry keys. */ -export const getPassStyleCover = passStyle => - PassStyleRankAndCover[PassStyleRank[passStyle]][1]; -harden(getPassStyleCover); +export const passStylePrefixes = harden({ + __proto__: null, + error: '!', + copyRecord: '(', + tagged: ':', + promise: '?', + copyArray: '[', + boolean: 'b', + number: 'f', + bigint: 'np', + remotable: 'r', + string: 's', + null: 'v', + symbol: 'y', + undefined: 'z', +}); diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js index 4f2cdede95..de01289ab1 100644 --- a/packages/marshal/src/rankOrder.js +++ b/packages/marshal/src/rankOrder.js @@ -2,14 +2,14 @@ import { getTag } from './helpers/passStyle-helpers.js'; import { passStyleOf } from './passStyleOf.js'; import { nameForPassableSymbol } from './helpers/symbol.js'; -import { PassStyleRank, recordParts } from './encodePassable.js'; +import { passStylePrefixes, recordParts } from './encodePassable.js'; /** @typedef {import('./types.js').Passable} Passable */ /** @typedef {import('./types.js').PassStyle} PassStyle */ /** @typedef {import('./types.js').RankCover} RankCover */ const { details: X, quote: q } = assert; -const { is } = Object; +const { entries, fromEntries, setPrototypeOf, is } = Object; /** * @typedef {-1 | 0 | 1} RankComparison @@ -97,6 +97,48 @@ const { is } = Object; */ const sameValueZero = (x, y) => x === y || is(x, y); +const trivialComparator = (left, right) => + // eslint-disable-next-line no-nested-ternary + left < right ? -1 : left === right ? 0 : 1; + +/** + * @typedef {Record} PassStyleRanksRecord + */ + +const passStyleRanks = /** @type {PassStyleRanksRecord} */ (fromEntries( + entries(passStylePrefixes) + // Sort entries by ascending prefix. + .sort(([_leftStyle, leftPrefixes], [_rightStyle, rightPrefixes]) => { + return trivialComparator(leftPrefixes, rightPrefixes); + }) + .map(([passStyle, prefixes], index) => { + // Cover all strings that start with any character in `prefixes`. + // `prefixes` is already sorted, so that's + // all s such that prefixes.at(0) ≤ s < successor(prefixes.at(-1)). + const cover = [ + prefixes.charAt(0), + String.fromCharCode(prefixes.charCodeAt(prefixes.length - 1) + 1), + ]; + return [passStyle, { index, cover }]; + }), +)); +setPrototypeOf(passStyleRanks, null); +harden(passStyleRanks); + +/** + * Associate with each passStyle a RankCover that may be an overestimate, + * and whose results therefore need to be filtered down. For example, because + * there is not a smallest or biggest bigint, bound it by `NaN` (the last place + * number) and `''` (the empty string, which is the first place string). Thus, + * a range query using this range may include these values, which would then + * need to be filtered out. + * + * @param {PassStyle} passStyle + * @returns {RankCover} + */ +export const getPassStyleCover = passStyle => passStyleRanks[passStyle].cover; +harden(getPassStyleCover); + /** * @type {WeakMap>} */ @@ -124,7 +166,10 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => { const leftStyle = passStyleOf(left); const rightStyle = passStyleOf(right); if (leftStyle !== rightStyle) { - return comparator(PassStyleRank[leftStyle], PassStyleRank[rightStyle]); + return trivialComparator( + passStyleRanks[leftStyle].index, + passStyleRanks[rightStyle].index, + ); } switch (leftStyle) { case 'remotable': { diff --git a/packages/marshal/test/test-rankOrder.js b/packages/marshal/test/test-rankOrder.js index de15076ca2..d896d502c3 100644 --- a/packages/marshal/test/test-rankOrder.js +++ b/packages/marshal/test/test-rankOrder.js @@ -8,10 +8,10 @@ import { compareRank, isRankSorted, sortByRank, + getPassStyleCover, getIndexCover, assertRankSorted, } from '../src/rankOrder.js'; -import { getPassStyleCover } from '../src/encodePassable.js'; import { makeTagged } from '../src/makeTagged.js'; import { Far } from '../src/make-far.js'; From 334abf6eefe460cc6af3a0621ef7f602b7f62fba Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sun, 6 Nov 2022 22:08:00 -0800 Subject: [PATCH 5/5] fix: faster record comparison --- packages/marshal/src/encodePassable.js | 44 ++++++++++++++++++-------- packages/marshal/src/rankOrder.js | 15 ++++++--- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/packages/marshal/src/encodePassable.js b/packages/marshal/src/encodePassable.js index 4b6d1b16a6..6e1ec35483 100644 --- a/packages/marshal/src/encodePassable.js +++ b/packages/marshal/src/encodePassable.js @@ -21,24 +21,39 @@ const { fromEntries, is } = Object; const { ownKeys } = Reflect; /** + * Assuming that `record` is a CopyRecord, we have only + * string-named own properties. `recordNames` returns those name *reverse* + * sorted, because that's how records are compared, encoded, and sorted. + * * @template T * @param {CopyRecord} record - * @returns {[string[], T[]]} + * @returns {string[]} */ -export const recordParts = record => { - assertRecord(record); +export const recordNames = record => // https://github.com/endojs/endo/pull/1260#discussion_r1003657244 - // compares two ways of reverse sorting, and shows that this way - // is currently faster on Moddable XS while the other way, + // compares two ways of reverse sorting, and shows that `.sort().reverse()` + // is currently faster on Moddable XS, while the other way, // `.sort(reverseComparator)`, is faster on v8. We currently care more about - // XS performance, so we reverse sort this way. - const names = /** @type {string[]} */ (ownKeys(record) - .sort() - .reverse()); - const vals = names.map(name => record[name]); - return harden([names, vals]); -}; -harden(recordParts); + // XS performance, so we reverse sort using `.sort().reverse()`. + harden( + /** @type {string[]} */ (ownKeys(record)) + .sort() + .reverse(), + ); +harden(recordNames); + +/** + * Assuming that `record` is a CopyRecord and `names` is `recordNames(record)`, + * return the corresponding array of property values. + * + * @template T + * @param {CopyRecord} record + * @param {string[]} names + * @returns {T[]} + */ +export const recordValues = (record, names) => + harden(names.map(name => record[name])); +harden(recordValues); export const zeroPad = (n, size) => { const nStr = `${n}`; @@ -236,7 +251,8 @@ const decodeArray = (encoded, decodePassable) => { }; const encodeRecord = (record, encodePassable) => { - const [names, values] = recordParts(record); + const names = recordNames(record); + const values = recordValues(record, names); return `(${encodeArray(harden([names, values]), encodePassable)}`; }; diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js index de01289ab1..3b1841ab56 100644 --- a/packages/marshal/src/rankOrder.js +++ b/packages/marshal/src/rankOrder.js @@ -2,7 +2,11 @@ import { getTag } from './helpers/passStyle-helpers.js'; import { passStyleOf } from './passStyleOf.js'; import { nameForPassableSymbol } from './helpers/symbol.js'; -import { passStylePrefixes, recordParts } from './encodePassable.js'; +import { + passStylePrefixes, + recordNames, + recordValues, +} from './encodePassable.js'; /** @typedef {import('./types.js').Passable} Passable */ /** @typedef {import('./types.js').PassStyle} PassStyle */ @@ -230,14 +234,17 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => { // of these names, which we then compare lexicographically. This ensures // that if the names of record X are a subset of the names of record Y, // then record X will have an earlier rank and sort to the left of Y. - const [leftNames, leftValues] = recordParts(left); - const [rightNames, rightValues] = recordParts(right); + const leftNames = recordNames(left); + const rightNames = recordNames(right); const result = comparator(leftNames, rightNames); if (result !== 0) { return result; } - return comparator(leftValues, rightValues); + return comparator( + recordValues(left, leftNames), + recordValues(right, rightNames), + ); } case 'copyArray': { // Lexicographic