Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(marshal): Replace more XS-expensive string operations #2005

Merged
merged 1 commit into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions packages/marshal/src/encodePassable.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ const encodeBinary64 = n => {
* @returns {number}
*/
const decodeBinary64 = encoded => {
encoded.startsWith('f') || Fail`Encoded number expected: ${encoded}`;
encoded.charAt(0) === 'f' || Fail`Encoded number expected: ${encoded}`;
let bits = BigInt(`0x${encoded.substring(1)}`);
if (encoded[1] < '8') {
bits ^= 0xffffffffffffffffn;
Expand Down Expand Up @@ -182,37 +182,38 @@ const encodeBigInt = n => {
}
};

const rBigIntPayload = /([0-9]+)(:([0-9]+$|)|)/s;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. Consolidating the regexp makes this much more declarative and readable. Nice!


/**
* @param {string} encoded
* @returns {bigint}
*/
const decodeBigInt = encoded => {
const typePrefix = encoded.charAt(0); // faster than encoded[0]
let rem = encoded.slice(1);
typePrefix === 'p' ||
typePrefix === 'n' ||
Fail`Encoded bigint expected: ${encoded}`;

const lDigits = rem.search(/[0-9]/) + 1;
lDigits >= 1 || Fail`Digit count expected: ${encoded}`;
rem = rem.slice(lDigits - 1);

rem.length >= lDigits || Fail`Complete digit count expected: ${encoded}`;
const snDigits = rem.slice(0, lDigits);
rem = rem.slice(lDigits);
/^[0-9]+$/.test(snDigits) || Fail`Decimal digit count expected: ${encoded}`;
const {
index: lDigits,
1: snDigits,
2: tail,
3: digits,
} = encoded.match(rBigIntPayload) || Fail`Digit count expected: ${encoded}`;

snDigits.length === lDigits ||
Fail`Unary-prefixed 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;
nDigits = 10 ** /** @type {number} */ (lDigits) - nDigits;
}

rem.startsWith(':') || Fail`Separator expected: ${encoded}`;
rem = rem.slice(1);
rem.length === nDigits ||
tail.charAt(0) === ':' || Fail`Separator expected: ${encoded}`;
digits.length === nDigits ||
Fail`Fixed-length digit sequence expected: ${encoded}`;
let n = BigInt(rem);
let n = BigInt(digits);
if (typePrefix === 'n') {
// TODO Assert to reject forbidden encodings
// like "n9:0" and "n8:00" and "n8:91" through "n8:99"?
Expand Down Expand Up @@ -292,7 +293,7 @@ const encodeRecord = (record, encodePassable) => {
};

const decodeRecord = (encoded, decodePassable) => {
assert(encoded.startsWith('('));
assert(encoded.charAt(0) === '(');
// Skip the "(" inside `decodeArray` to avoid slow `substring` in XS.
// https://github.com/endojs/endo/issues/1984
const unzippedEntries = decodeArray(encoded, decodePassable, 1);
Expand All @@ -314,7 +315,7 @@ const encodeTagged = (tagged, encodePassable) =>
`:${encodeArray(harden([getTag(tagged), tagged.payload]), encodePassable)}`;

const decodeTagged = (encoded, decodePassable) => {
assert(encoded.startsWith(':'));
assert(encoded.charAt(0) === ':');
// Skip the ":" inside `decodeArray` to avoid slow `substring` in XS.
// https://github.com/endojs/endo/issues/1984
const taggedPayload = decodeArray(encoded, decodePassable, 1);
Expand Down Expand Up @@ -378,19 +379,19 @@ export const makeEncodePassable = (encodeOptions = {}) => {
}
case 'remotable': {
const result = encodeRemotable(passable, encodePassable);
result.startsWith('r') ||
result.charAt(0) === 'r' ||
Fail`internal: Remotable encoding must start with "r": ${result}`;
return result;
}
case 'error': {
const result = encodeError(passable, encodePassable);
result.startsWith('!') ||
result.charAt(0) === '!' ||
Fail`internal: Error encoding must start with "!": ${result}`;
return result;
}
case 'promise': {
const result = encodePromise(passable, encodePassable);
result.startsWith('?') ||
result.charAt(0) === '?' ||
Fail`internal: Promise encoding must start with "?": ${result}`;
return result;
}
Expand Down
8 changes: 4 additions & 4 deletions packages/marshal/src/encodeToSmallcaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export const makeEncodeToSmallcaps = (encodeOptions = {}) => {
// Assert that the #error property decodes to a string.
const message = encoding['#error'];
(typeof message === 'string' &&
(!startsSpecial(message) || message.startsWith('!'))) ||
(!startsSpecial(message) || message.charAt(0) === '!')) ||
Fail`internal: Error encoding must have string message: ${q(message)}`;
};

Expand Down Expand Up @@ -241,7 +241,7 @@ export const makeEncodeToSmallcaps = (encodeOptions = {}) => {
passable,
encodeToSmallcapsRecur,
);
if (typeof result === 'string' && result.startsWith('$')) {
if (typeof result === 'string' && result.charAt(0) === '$') {
return result;
}
// `throw` is noop since `Fail` throws. But linter confused
Expand All @@ -252,7 +252,7 @@ export const makeEncodeToSmallcaps = (encodeOptions = {}) => {
passable,
encodeToSmallcapsRecur,
);
if (typeof result === 'string' && result.startsWith('&')) {
if (typeof result === 'string' && result.charAt(0) === '&') {
return result;
}
throw Fail`internal: Promise encoding must start with "&": ${result}`;
Expand Down Expand Up @@ -446,7 +446,7 @@ export const makeDecodeFromSmallcaps = (decodeOptions = {}) => {
Fail`Property name ${q(
encodedName,
)} of ${encoding} must be a string`;
!encodedName.startsWith('#') ||
encodedName.charAt(0) !== '#' ||
Fail`Unrecognized record type ${q(encodedName)}: ${encoding}`;
const name = decodeFromSmallcaps(encodedName);
typeof name === 'string' ||
Expand Down
4 changes: 2 additions & 2 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ export const makeMarshal = (
* @returns {Remotable | Promise}
*/
return (stringEncoding, _decodeRecur) => {
assert(stringEncoding.startsWith(prefix));
assert(stringEncoding.charAt(0) === prefix);
// slots: $slotIndex.iface or $slotIndex
const i = stringEncoding.indexOf('.');
const index = Number(stringEncoding.slice(1, i < 0 ? undefined : i));
Expand Down Expand Up @@ -350,7 +350,7 @@ export const makeMarshal = (
const { reviveFromCapData, reviveFromSmallcaps } = makeFullRevive(slots);
let result;
// JSON cannot begin with a '#', so this is an unambiguous signal.
if (body.startsWith('#')) {
if (body.charAt(0) === '#') {
const smallcapsBody = body.slice(1);
const encoding = harden(JSON.parse(smallcapsBody));
result = harden(reviveFromSmallcaps(encoding));
Expand Down
Loading