fix(xsnap): upgrade to latest xsnap, now with timestamps, and update METER_TYPE #5598
Conversation
warner
force-pushed
the
5152-add-xsnap-timestamps
branch
3 times, most recently
from
28cac3a
to
94b7a28
Compare
|
|
||
| t.deepEqual(sortedNames, expected); | ||
|
|
||
| // on my 2022 MBP (M1 Pro), syscalls take 75-600us to get from |
There was a problem hiding this comment.
I like to t.log() such numbers so I can eyeball them with ava test -v.
warner
force-pushed
the
5152-add-xsnap-timestamps
branch
from
94b7a28
to
aa4be60
Compare
PR #5338 updated our copy of the XS SDK to the most recent version. That might have changed the metering behavior, so we need to update METER_TYPE from `xs-meter-13` to `xs-meter-14`, so downstream users know that computron counts aren't necessarily compatible across this change.
This moves to the version of `xsnap-worker.c` which includes receipt/transmit timestamps in the metering results. Tests are updated to expect 'timestamps' in xsnap metering results, and to assert that they correctly interleave with the times of upstream/downstream commands as seen by the parent process. refs #5152
warner
force-pushed
the
5152-add-xsnap-timestamps
branch
from
aa4be60
to
c857cc8
Compare
| @@ -3,7 +3,7 @@ | |||
| /** The version identifier for our meter type. | |||
| * TODO Bump this whenever there's a change to metering semantics. | |||
| */ | |||
| export const METER_TYPE = 'xs-meter-13'; | |||
| export const METER_TYPE = 'xs-meter-14'; | |||
There was a problem hiding this comment.
What does "a change to metering semantics" mean exactly? What is the purpose of this bump? Should this be automated, or at least tested so that an effective change to XS is represented here?
There was a problem hiding this comment.
indeed, there is a test:
agoric-sdk/packages/xsnap/test/test-xs-perf.js
Lines 49 to 53 in 102a5b6
There was a problem hiding this comment.
Interesting, I don't see what in this PR causes the need to update the meter type.
There was a problem hiding this comment.
Perhaps the addition of timestamps was, conservatively, considered to be a change in the metering semantics. Note the test for the shape of the meters object just after the test for magnitudes.
fix(xsnap): upgrade to latest xsnap, now with timestamps
This moves to the version of
xsnap-worker.cwhich includesreceipt/transmit timestamps in the metering results.
Tests are updated to expect 'timestamps' in xsnap metering results,
and to assert that they correctly interleave with the times of
upstream/downstream commands as seen by the parent process.
refs #5152