endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

refactor(ses): Improve the performance of error serialization #1865

Closed gibson042 closed 10 months ago

gibson042 commented 11 months ago

Description

Noticed in passing: building a string by successive concatenation rather than pushing into an eventually-joined array is faster in both V8 (+63%) and XS (+39%), presumably because of rope optimizations.

```console $ esbench --eshost-option '-h V8,*XS*' ' const { raw: StringRaw } = String; const [arrayIncludes, arrayMap, arrayPush, arrayJoin] = ["includes", "map", "push", "join"].map( m => Function.prototype.call.bind(Array.prototype[m]), ); const useRaw = (template, ...args) => StringRaw({ raw: template }, ...arrayMap(args, arg => `${arg}`)); const useJoin = (template, ...args) => { const parts = [template[0]]; for (let i = 0; i < args.length; i += 1) { const arg = args[i]; const argStr = `${arg}`; arrayPush(parts, argStr, template[i + 1]); } return arrayJoin(parts, ""); }; const usePlus = (template, ...args) => { let result = `${template[0]}`; for (let i = 0; i < args.length; i += 1) { const arg = args[i]; const argStr = `${arg}`; result += argStr + template[i + 1]; } return result; }; const useTemplates = (template, ...args) => { let result = `${template[0]}`; for (let i = 0; i < args.length; i += 1) { const arg = args[i]; const argStr = `${arg}`; result += `${argStr}${template[i + 1]}`; } return result; }; for (const [name, fn] of Object.entries({ useRaw, useJoin, usePlus, useTemplates })) { const actual = fn`a ${0.0} ${1e0}`; const expected = "a 0 1"; if (actual === expected) continue; print(JSON.stringify({ actual, expected })); throw Error(`bad ${name}`); }' '{ useRaw: "result = useRaw`problem ${0}: ${[`blue`, 42]}`", useJoin: "result = useJoin`problem ${0}: ${[`blue`, 42]}`", usePlus: "result = usePlus`problem ${0}: ${[`blue`, 42]}`", useTemplates: "result = useTemplates`problem ${0}: ${[`blue`, 42]}`", }' ```

Moddable XS

useRaw: 0.61 ops/ms useJoin: 0.62 ops/ms usePlus: 0.86 ops/ms useTemplates: 0.72 ops/ms

V8

useRaw: 2.72 ops/ms useJoin: 4.65 ops/ms usePlus: 7.58 ops/ms useTemplates: 7.35 ops/ms

Security Considerations

n/a

Scaling Considerations

:+1:

Documentation Considerations

n/a

Testing Considerations

all tests continue to pass

Upgrade Considerations

None beyond the usual concerns about changing vat bundles.

erights commented 11 months ago

and XS (+39%), presumably because of rope optimizations.

But XS doesn't use ropes.

@phoddie , shouldn't we generally use repeated array push + join in XS rather than repeated string appends? Does it make sense the latter could be faster?

phoddie commented 11 months ago

@erights – Correct, XS doesn't use ropes. Perhaps for strings created for a smaller number of items, using string concatenation is faster but as the number of items grows, the array push + join approach is faster?

erights commented 10 months ago

If not, I suggest that this PR be closed in favor of the status quo.

Close?

gibson042 commented 10 months ago

Given the above, it seems like small-input performance improvements do not provide sufficient benefit to justify introducing the pattern.