fix(marshal): Add a smaller and more efficient passable encoding #1594

gibson042 · 2023-05-23T20:24:10Z

Open questions:

[REJECTED] Should we push for extra compactness, e.g. bfalse → bf/btrue → bt (booleans) and/or p1:0 → o (0n)?
[REJECTED] Should we allow custom remotable/promise/error encoding to use sequences and/or characters that would not otherwise be allowed, such as raw control characters U+0000 through U+001F or raw string escape characters !/_?
[REPLACE ALL C0 CONTROLS] For that matter, should we replace the full U+0000 through U+001F range in strings, or just a subset? The full range is necessary if U+0020 SPACE is the array element separator, but that could alternatively be U+0009 TAB and escapes could be limited to code points at or before that. This is really a tradeoff around expectations of character frequency inside strings, limiting blowup when embedding encodings in JSON-serialized containers such as current CapData communication, and (to some extent) human comprehensibility. There's not really a performance difference and at most a linear compactness difference, so it's really a question of aesthetics—for example, JSON-serializing the encoding of { "metasyntactic variables": ["foo", "bar", "baz"], tab: "\t", null: "\0" } would change as follows if we used tab-termination rather than space-termination:
```
-"~(^^stab snull smetasyntactic!_variables  ^s!* s!! ^sfoo sbar sbaz   "
+"~(^^stab\tsnull\tsmetasyntactic variables\t\t^s\n9\ts\n0\t^sfoo\tsbar\tsbaz\t\t\t"
```

packages/marshal/src/encodePassable.js

gibson042 · 2023-05-23T20:34:15Z

packages/marshal/src/encodePassable.js

@@ -328,11 +451,17 @@ export const makeEncodePassable = (encodeOptions = {}) => {
    encodeRemotable = (rem, _) => Fail`remotable unexpected: ${rem}`,
    encodePromise = (prom, _) => Fail`promise unexpected: ${prom}`,
    encodeError = (err, _) => Fail`error unexpected: ${err}`,
+    xxx = false,


We need a name for the smaller encoding and this property opting in to it, and ideally also a name for the current encoding. I don't have any good suggestions at the moment, so 🚲🏠 away!

Some thoughts:

oldAndBusted vs newHawtness

tired vs wired

escapism vs realism

legacyOrdered vs compactOrdered ?

For what it's worth, I don't think there's a fundamental aspect to the new encoding other than translation of rank ordering into lexicographic ordering (a feature which it shares with the current encoding). And the key detail differentiating the two is that the current encoding applies escaping to array elements (causing blowup for deep structures) while the new one applies escaping to strings and transitively to symbols, which are the only types that can contain characters confusable with array encoding delimiters and as scalars are immune to the blowup issue.

"Compact" is accurate in a relative sense, but keep in mind that we're still using text rather than raw octets.

So legacyOrdered vs compactOrdered is fine?

I would think just "legacy" vs. "compact", e.g. makeEncodePassable({ format: "compact" }). What significance of "ordered" am I missing?

Neither smallcaps nor its predecessor are order preserving. The main feature of both the formats we're naming here is that they are.

Ah, so IIUC you're thinking about something like makeMarshal(convertValToSlot, convertSlotToVal, { serializeBodyFormat: "compactOrdered" }) and wanting "compactOrdered" to be used everywhere. 👍

yes, exactly.

packages/marshal/src/encodePassable.js

erights · 2023-06-23T03:54:27Z

Hopefully fixes Agoric/agoric-sdk#2589 , yes?

erights

Sorry this took so long!

Looking forward to using this! I think I can start making use of this soon.

packages/marshal/src/encodePassable.js

erights · 2023-08-06T21:16:31Z

packages/marshal/src/encodePassable.js

@@ -328,11 +451,17 @@ export const makeEncodePassable = (encodeOptions = {}) => {
    encodeRemotable = (rem, _) => Fail`remotable unexpected: ${rem}`,
    encodePromise = (prom, _) => Fail`promise unexpected: ${prom}`,
    encodeError = (err, _) => Fail`error unexpected: ${err}`,
+    xxx = false,


legacyOrdered vs compactOrdered ?

packages/marshal/src/encodePassable.js

erights · 2023-08-07T00:58:34Z

packages/marshal/src/encodePassable.js

+  const encodeString = xxx
+    ? encodeStringWithEscapes
+    : encodeStringWithoutEscapes;
+  const encodeArray = xxx ? encodeArrayWithoutEscapes : encodeArrayWithEscapes;


I was initially very confused until I realized that encodeStringWithEscapes goes with encodeArrayWithoutEscapes and encodeStringWithoutEscapes goes with encodeArrayWithEscapes. Once we have a name for xxx, then perhaps use that in these function names so it is less surprising which goes with which.

packages/marshal/src/encodePassable.js

erights · 2023-08-18T02:42:22Z

Btw @gibson042 , I reviewed it as if it is Ready For Review. Is there a reason it is still Draft?

erights · 2023-08-18T02:42:48Z

Any idea why CI didn't complete?

gibson042 · 2023-08-26T17:07:31Z

Hopefully fixes Agoric/agoric-sdk#2589 , yes?

Yes, I think we can make that claim.

Btw @gibson042 , I reviewed it as if it is Ready For Review. Is there a reason it is still Draft?

I consider it a draft until we settle on a name.

Any idea why CI didn't complete?

Pushed to kick CI, and I can also rebase on master if that won't disrupt review.

erights · 2023-08-26T23:42:32Z

and I can also rebase on master if that won't disrupt review

Please do rebase to master, thanks.

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

…"compactOrdered") #1594 (comment)

gibson042 · 2023-09-07T21:23:55Z

Updated to use "legacyOrdered" and "compactOrdered" names—so this PR is no longer a draft!

erights

LGTM, thanks!

(If possible, would be nice to see this merged before tomorrow's ocapn mtg)

gibson042 · 2023-09-12T20:05:59Z

Returning to draft because I found that the new format needs more handling for output from functions provided to makeEncodePassable:

import "@endo/init";
import { Far } from "@endo/far";
import { makeMarshal, makeEncodePassable } from "@endo/marshal";
const r0 = Far("Foo");
const data = harden([r0, r0, "."]);
const safeEncodeRemotable = r => "r0";
const unsafeEncodeRemotable = r => "r 0";
const baseOptions = { format: "compactOrdered" };
console.log({
  safeEncoding: makeEncodePassable({ ...baseOptions, encodeRemotable: safeEncodeRemotable })(data),
  unsafeEncoding: makeEncodePassable({ ...baseOptions, encodeRemotable: unsafeEncodeRemotable })(data),
});
/* =>
{
  safeEncoding: '~^r0 r0 s. ',
  unsafeEncoding: '~^r 0 r 0 s. '
}
*/

gibson042 · 2023-09-12T22:42:01Z

Returning to draft because I found that the new format needs more handling for output from functions provided to makeEncodePassable

Resolved now; @erights please review the updates at 9c9d1e3 .

gibson042 · 2023-09-12T22:43:27Z

packages/marshal/src/encodePassable.js

@@ -346,7 +567,7 @@ export const makeEncodePassable = (encodeOptions = {}) => {
        return encodeBinary64(passable);
      }
      case 'string': {
-        return `s${passable}`;
+        return `s${encodeStringSuffix(passable)}`;
      }
      case 'boolean': {
        return `b${passable}`;


While we're making changes, should we also replace "btrue" and "bfalse" with e.g. "bt" and "bf"?

b1 and b0?

packages/marshal/src/encodePassable.js

erights · 2023-09-13T21:01:36Z

Could we provide it such a decode function?

…

On Wed, Sep 13, 2023 at 5:11 AM Richard Gibson ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/marshal/src/encodePassable.js <#1594 (comment)>: > + } else if (format === 'compactOrdered') { + formatPrefix = '~'; + encodeStringSuffix = encodeCompactStringSuffix; + encodeArray = encodeCompactArray; + } else { + Fail`Unrecognized format: ${q(format)}`; + } + const encodeRemotable = makeEncodeRemotable( + unsafeEncodeRemotable, + encodeStringSuffix, + ); + const encodePromise = makeEncodePromise( + unsafeEncodePromise, + encodeStringSuffix, + ); + const encodeError = makeEncodeError(unsafeEncodeError, encodeStringSuffix); This double-escapes the contents of any embedded string (such as for the error message), but I don't see a good alternative. In particular, asserting that unsafeEncodeError produced an independently valid encoding that needs no further escaping would be awkward because this code doesn't have access to any caller-provided decode functions for embedded remotables/promises/errors. — Reply to this email directly, view it on GitHub <#1594 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACC3TAZEZWGDV3GPWD3SW3X2GPH7ANCNFSM6AAAAAAYMMXCXA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

…"compactOrdered") #1594 (comment)

warner · 2024-01-17T19:21:04Z

Any chance we could get a neighboring .md document that explains the format?

This document would also be a good place to mention that the format: option limits the decoder's expectations.. if I read this correctly, there's no option to build a decoder that can handle both new- and old- format data. To use this in liveslots, we'll either need:

1: Set a vat-wide flag to say what format we're using for all virtual collection keys. If we're launching a brand new vat, we can set it to use the new format forever. If the new-format-capable liveslots is loaded in a pre-existing vat (ie vat upgrade), and it doesn't see this flag, it must stick with the old format forever.
or 2: Introduce some sort of prefix in the vatstore records to indicate whether each record is new- or old- format. The current vatstore key is like vc.5.sfoo (where vc means "virtual collection", 5 means collection number 5, and sfoo is the passable-encoded string key foo). If there's a format key (s/n/p/b/r/etc) that's guaranteed to not appear in the old format, we could use that as a per-record indicator, but it would break sort order between old- and new- format keys of the same category. So that doesn't leave us much/any room for a per-record format indicator, but maybe we could do a per-collection indicator, and at least allow collections created after the upgrade to use the new format.

mhofman · 2024-01-17T20:19:47Z

Introduce some sort of prefix in the vatstore records to indicate whether each record is new- or old- format.

I expect this to be per collection to preserve sort order between entries, so it could be in the vc metadata.

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

…"compactOrdered") #1594 (comment)

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

…"compactOrdered") #1594 (comment)

Fixes #1349

Fixes failures on Node.js v14

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

…motable/promise/error callbacks

…"compactOrdered") #1594 (comment)

…dered format

…ble/promise/error compactOrdered encodings (rather than forcing everything through string encoding)

… escaped

… remotable/promise/error compactOrdered encodings

* Introduce a short-circuiting `getSuffix` (for skip=0). * Extend "skip" to `innerDecode`, `decodeBinary64`, `decodeRecord`, and `decodeTagged`.

gibson042 requested a review from erights May 23, 2023 20:24

gibson042 marked this pull request as draft May 23, 2023 20:24

gibson042 commented May 23, 2023

View reviewed changes

erights mentioned this pull request May 25, 2023

Concrete syntax ordering guarantees ocapn/ocapn#56

Open

erights mentioned this pull request Jun 1, 2023

WIP: Event-based encoding API based on Data-E #1423

Draft

gibson042 mentioned this pull request Jun 13, 2023

Proposed wire format ocapn/ocapn#63

Open

erights mentioned this pull request Jun 23, 2023

replace JSON-based marshal with dense binary encoding Agoric/agoric-sdk#2589

Open

erights requested changes Aug 7, 2023

View reviewed changes

gibson042 force-pushed the gibson-1349-encodepassable-escaping branch from 0cee48b to 402bb08 Compare August 27, 2023 16:12

gibson042 mentioned this pull request Aug 28, 2023

For performance, compactOrdered decodePassable should minimize reparsing #1747

Open

gibson042 added a commit that referenced this pull request Aug 29, 2023

refactor(marshal): Change new-encoding prefix symbol from "#" to "~"

3f39f36

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

gibson042 added a commit that referenced this pull request Sep 7, 2023

refactor(marshal): Change new-encoding prefix symbol from "#" to "~"

79ef761

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

gibson042 added a commit that referenced this pull request Sep 7, 2023

chore(marshal): Name the encodePassable formats ("legacyOrdered" and …

f2c1df6

…"compactOrdered") #1594 (comment)

gibson042 force-pushed the gibson-1349-encodepassable-escaping branch from c97046e to f2c1df6 Compare September 7, 2023 21:23

gibson042 marked this pull request as ready for review September 7, 2023 22:13

erights approved these changes Sep 11, 2023

View reviewed changes

gibson042 marked this pull request as draft September 12, 2023 19:59

gibson042 marked this pull request as ready for review September 12, 2023 22:42

gibson042 commented Sep 12, 2023

View reviewed changes

erights self-requested a review September 13, 2023 02:33

gibson042 commented Sep 13, 2023

View reviewed changes

packages/marshal/src/encodePassable.js Outdated Show resolved Hide resolved

gibson042 added a commit that referenced this pull request Jan 12, 2024

chore(marshal): Name the encodePassable formats ("legacyOrdered" and …

c23475d

…"compactOrdered") #1594 (comment)

gibson042 force-pushed the gibson-1349-encodepassable-escaping branch from 85a996c to a99748a Compare January 12, 2024 16:33

gibson042 added a commit that referenced this pull request Jan 18, 2024

refactor(marshal): Change new-encoding prefix symbol from "#" to "~"

751f59f

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

gibson042 added a commit that referenced this pull request Jan 18, 2024

chore(marshal): Name the encodePassable formats ("legacyOrdered" and …

90189c7

…"compactOrdered") #1594 (comment)

gibson042 force-pushed the gibson-1349-encodepassable-escaping branch from a99748a to 8f93653 Compare January 18, 2024 01:04

gibson042 mentioned this pull request Jan 24, 2024

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

Merged

gibson042 added a commit that referenced this pull request Jan 25, 2024

refactor(marshal): Change new-encoding prefix symbol from "#" to "~"

19777aa

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

gibson042 added a commit that referenced this pull request Jan 25, 2024

chore(marshal): Name the encodePassable formats ("legacyOrdered" and …

759e3f6

…"compactOrdered") #1594 (comment)

gibson042 force-pushed the gibson-1349-encodepassable-escaping branch from 8f93653 to 17601ac Compare January 25, 2024 06:31

gibson042 added 14 commits February 5, 2024 10:26

fix(marshal): Add a smaller and more efficient passable encoding

a85616f

Fixes #1349

test(marshal): Make a macro for encodePassable round-trip tests

d90dfe2

refactor(marshal): Use String#replace rather than replaceAll

961c7c3

Fixes failures on Node.js v14

chore(marshal): Improve comments

52dbda6

refactor(marshal): Change new-encoding prefix symbol from "#" to "~"

242d905

This provides better differention w.r.t. smallcaps, cf. #1594 (comment)

chore(marshal): Consistently assert behavior of makeEncodePassable re…

4ed2c15

…motable/promise/error callbacks

chore(marshal): Name the encodePassable formats ("legacyOrdered" and …

820932a

…"compactOrdered") #1594 (comment)

chore(marshal): Escape remotable/promise/error encodings in compactOr…

1486557

…dered format

feat(marshal): Introduce makePassableKit and support arbitrary remota…

043039f

…ble/promise/error compactOrdered encodings (rather than forcing everything through string encoding)

chore(marshal): Reject strings containing characters that *should* be…

0afce72

… escaped

chore(marshal): Disallow C0 control characters in custom encodings

d6abd2c

fixup! feat(marshal): Introduce makePassableKit and support arbitrary…

52a67f9

… remotable/promise/error compactOrdered encodings

perf(marshal): Avoid even more substring operations

59b02aa

* Introduce a short-circuiting `getSuffix` (for skip=0). * Extend "skip" to `innerDecode`, `decodeBinary64`, `decodeRecord`, and `decodeTagged`.

docs(marshal): Start documenting passable encoding

70bcca3

gibson042 force-pushed the gibson-1349-encodepassable-escaping branch from 17601ac to 70bcca3 Compare February 5, 2024 16:58

gibson042 enabled auto-merge February 5, 2024 16:59

gibson042 merged commit f721c3a into master Feb 5, 2024
14 checks passed

gibson042 deleted the gibson-1349-encodepassable-escaping branch February 5, 2024 17:06

gibson042 mentioned this pull request Oct 21, 2024

Should encodePassable format "compactOrdered" not use "~" as an encoding prefix? #2600

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(marshal): Add a smaller and more efficient passable encoding #1594

fix(marshal): Add a smaller and more efficient passable encoding #1594

gibson042 commented May 23, 2023 •

edited

Loading

gibson042 May 23, 2023 •

edited

Loading

kriskowal Jun 23, 2023

kriskowal Jun 23, 2023

erights Aug 6, 2023

gibson042 Aug 26, 2023 •

edited

Loading

erights Aug 28, 2023

gibson042 Aug 28, 2023

erights Aug 28, 2023

gibson042 Aug 29, 2023

erights Aug 29, 2023

erights commented Jun 23, 2023

erights left a comment

erights Aug 6, 2023

erights Aug 7, 2023

erights commented Aug 18, 2023

erights commented Aug 18, 2023

gibson042 commented Aug 26, 2023

erights commented Aug 26, 2023

gibson042 commented Sep 7, 2023

erights left a comment

gibson042 commented Sep 12, 2023 •

edited

Loading

gibson042 commented Sep 12, 2023

gibson042 Sep 12, 2023

warner Jan 17, 2024

erights commented Sep 13, 2023 via email

warner commented Jan 17, 2024

mhofman commented Jan 17, 2024

fix(marshal): Add a smaller and more efficient passable encoding #1594

fix(marshal): Add a smaller and more efficient passable encoding #1594

Conversation

gibson042 commented May 23, 2023 • edited Loading

gibson042 May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 Aug 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erights commented Jun 23, 2023

erights left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erights commented Aug 18, 2023

erights commented Aug 18, 2023

gibson042 commented Aug 26, 2023

erights commented Aug 26, 2023

gibson042 commented Sep 7, 2023

erights left a comment

Choose a reason for hiding this comment

gibson042 commented Sep 12, 2023 • edited Loading

gibson042 commented Sep 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erights commented Sep 13, 2023 via email

warner commented Jan 17, 2024

mhofman commented Jan 17, 2024

gibson042 commented May 23, 2023 •

edited

Loading

gibson042 May 23, 2023 •

edited

Loading

gibson042 Aug 26, 2023 •

edited

Loading

gibson042 commented Sep 12, 2023 •

edited

Loading