refactor(pass-style): faster passStyleOf (#2716)

Closes: #XXXX
Refs: Agoric/agoric-sdk#10975
Agoric/agoric-sdk#10982

## Description

Using the benchmark tool from #10975 by @muhammadahmadasifbhatti as
modified by #10982 to produce flamegraphs (interactive in vscode) I
iterated on that benchmark test which already had very specific simple
examples exercising `passStyleOf`. Together with the snippet at
https://github.com/Agoric/agoric-sdk/blob/acbb5da3c5a52bab8db319fd696aed70146f9a89/.github/actions/restore-node/action.yml#L156
(which @gibson042 brought to my attention)

```sh
$ scripts/get-packed-versions.sh ~/endo | scripts/resolve-versions.sh
$ yarn install && yarn build
$ cd packages/benchmark-test
$ yarn bench
```
and then clicking on the latest *.cpuprofile file, assuming you've
already installed the
https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-js-profile-flame
vscode extension.

I was able to iterate and tinker on possible improvements, to see what
made how much of a difference. This PR has the improvements to
`passStyleOf` that I and my reviewers have came up with so far.

@gibson042 suggested the main technique used: to avoid redundant
checking by breaking up `assertValid` so we could still running the
checks of `canBeValid` twice.

### Security Considerations

I claim that this PR is a pure refactoring. If true, none.
***Reviewers***, please review with a skeptical eye. `passStyleOf` is
security critical, so any observable difference might lead to an
opportunity for an adversary.

### Scaling Considerations

Even though I iterated on a specialized benchmark exercising
`passStyleOf`, I believe I only made changes that would also be an
improvement for the typical cases.

### Documentation Considerations

If this is a pure refactor, none.

### Testing Considerations

If this is a pure refactor, none. In fact, this PR did not need to
change any tests, providing some weak evidence that it is indeed a pure
refactor.

### Compatibility Considerations

If this is a pure refactor, none.

### Upgrade Considerations

If this is a pure refactor, none.

packages/pass-style/src/copyArray.js

@@ -7,26 +7,18 @@ const { getPrototypeOf } = Object;
 const { ownKeys } = Reflect;
 const { isArray, prototype: arrayPrototype } = Array;
 
-/**
- * @param {unknown} candidate
- * @param {import('./types.js').Checker} [check]
- * @returns {boolean}
- */
-const canBeValid = (candidate, check = undefined) =>
-  isArray(candidate) ||
-  (!!check && check(false, X`Array expected: ${candidate}`));
-
 /**
  *
  * @type {import('./internal-types.js').PassStyleHelper}
  */
 export const CopyArrayHelper = harden({
   styleName: 'copyArray',
 
-  canBeValid,
+  canBeValid: (candidate, check = undefined) =>
+    isArray(candidate) ||
+    (!!check && check(false, X`Array expected: ${candidate}`)),
 
-  assertValid: (candidate, passStyleOfRecur) => {
-    canBeValid(candidate, assertChecker);
+  assertRestValid: (candidate, passStyleOfRecur) => {
     getPrototypeOf(candidate) === arrayPrototype ||
       assert.fail(X`Malformed array: ${candidate}`, TypeError);
     // Since we're already ensured candidate is an array, it should not be

packages/pass-style/src/copyRecord.js

@@ -64,19 +64,17 @@ export const CopyRecordHelper = harden({
     );
   },
 
-  assertValid: (candidate, passStyleOfRecur) => {
-    checkObjectPrototype(candidate, assertChecker);
-
-    // Validate that each own property is appropriate, data/enumerable,
-    // and has a recursively passable associated value.
+  assertRestValid: (candidate, passStyleOfRecur) => {
+    // Validate that each own property has a recursively passable associated
+    // value (we already know from canBeValid that the other constraints are
+    // satisfied).
     for (const name of ownKeys(candidate)) {
       const { value } = getOwnDataDescriptor(
         candidate,
         name,
         true,
         assertChecker,
       );
-      checkPropertyCanBeValid(candidate, name, value, assertChecker);
       passStyleOfRecur(value);
     }
   },

packages/pass-style/src/error.js

@@ -208,6 +208,6 @@ export const ErrorHelper = harden({
 
   canBeValid: checkErrorLike,
 
-  assertValid: (candidate, passStyleOfRecur) =>
+  assertRestValid: (candidate, passStyleOfRecur) =>
     checkRecursivelyPassableError(candidate, passStyleOfRecur, assertChecker),
 });

packages/pass-style/src/internal-types.js

@@ -20,9 +20,9 @@ export {};
  * @property {(candidate: any, check?: Checker) => boolean} canBeValid
  * If `canBeValid` returns true, then the candidate would
  * definitely not be valid for any of the other helpers.
- * `assertValid` still needs to be called to determine if it
- * actually is valid.
+ * `assertRestValid` still needs to be called to determine if it
+ * actually is valid, but only after the `canBeValid` check has passed.
  * @property {(candidate: any,
  *             passStyleOfRecur: (val: any) => PassStyle
- *            ) => void} assertValid
+ *            ) => void} assertRestValid
  */

packages/pass-style/src/passStyleOf.js

@@ -4,7 +4,12 @@
 
 import { isPromise } from '@endo/promise-kit';
 import { X, Fail, q, annotateError, makeError } from '@endo/errors';
-import { isObject, isTypedArray, PASS_STYLE } from './passStyle-helpers.js';
+import {
+  isObject,
+  isTypedArray,
+  PASS_STYLE,
+  assertChecker,
+} from './passStyle-helpers.js';
 
 import { CopyArrayHelper } from './copyArray.js';
 import { CopyRecordHelper } from './copyRecord.js';
@@ -63,6 +68,21 @@ const makeHelperTable = passStyleHelpers => {
   return harden(HelperTable);
 };
 
+/**
+ * The `assertRestValid` assumes that the `canBeValid` check has already passed.
+ * Contexts where we cannot assume that should call `assertValid` instead,
+ * which checks both conditions in the right order.
+ *
+ * @param {PassStyleHelper} helper
+ * @param {any} candidate
+ * @param {(val: any) => PassStyle} passStyleOfRecur
+ * @returns {void}
+ */
+const assertValid = (helper, candidate, passStyleOfRecur) => {
+  helper.canBeValid(candidate, assertChecker);
+  helper.assertRestValid(candidate, passStyleOfRecur);
+};
+
 /**
  * @param {PassStyleHelper[]} passStyleHelpers The passStyleHelpers to register,
  * in priority order.
@@ -164,24 +184,24 @@ const makePassStyleOf = passStyleHelpers => {
             const helper = HelperTable[passStyleTag];
             helper !== undefined ||
               Fail`Unrecognized PassStyle: ${q(passStyleTag)}`;
-            helper.assertValid(inner, passStyleOfRecur);
+            assertValid(helper, inner, passStyleOfRecur);
             return /** @type {PassStyle} */ (passStyleTag);
           }
           for (const helper of passStyleHelpers) {
             if (helper.canBeValid(inner)) {
-              helper.assertValid(inner, passStyleOfRecur);
+              helper.assertRestValid(inner, passStyleOfRecur);
               return helper.styleName;
             }
           }
-          remotableHelper.assertValid(inner, passStyleOfRecur);
+          assertValid(remotableHelper, inner, passStyleOfRecur);
           return 'remotable';
         }
         case 'function': {
           isFrozen(inner) ||
             Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
           typeof inner.then !== 'function' ||
             Fail`Cannot pass non-promise thenables`;
-          remotableHelper.assertValid(inner, passStyleOfRecur);
+          assertValid(remotableHelper, inner, passStyleOfRecur);
           return 'remotable';
         }
         default: {

packages/pass-style/src/remotable.js

@@ -272,7 +272,7 @@ export const RemotableHelper = harden({
     return !!check && CX(check)`unrecognized typeof ${candidate}`;
   },
 
-  assertValid: candidate => checkRemotable(candidate, assertChecker),
+  assertRestValid: candidate => checkRemotable(candidate, assertChecker),
 
   every: (_passable, _fn) => true,
 });

packages/pass-style/src/tagged.js

@@ -9,20 +9,24 @@ import {
   checkPassStyle,
 } from './passStyle-helpers.js';
 
+/**
+ * @import {PassStyleHelper} from './internal-types.js'
+ */
+
 const { ownKeys } = Reflect;
 const { getOwnPropertyDescriptors } = Object;
 
 /**
  *
- * @type {import('./internal-types.js').PassStyleHelper}
+ * @type {PassStyleHelper}
  */
 export const TaggedHelper = harden({
   styleName: 'tagged',
 
   canBeValid: (candidate, check = undefined) =>
     checkPassStyle(candidate, candidate[PASS_STYLE], 'tagged', check),
 
-  assertValid: (candidate, passStyleOfRecur) => {
+  assertRestValid: (candidate, passStyleOfRecur) => {
     checkTagRecord(candidate, 'tagged', assertChecker);
 
     // Typecasts needed due to https://github.com/microsoft/TypeScript/issues/1863