feat(bench-test): Benchmark Testing

[muhammadahmadasifbhatti]

Note: This PR is migrated from the agoric-sdk. All the comments from Mark's reviewed PR are addressed.

Description

This is PR is just a minimalistic POC to create a small tool for the benchmark testing.

How to run locally

Run the command yarn test in the packages/benchmark-test folder.

Pre-reqs of running yarn test

Install esvu globally

Select the v8 and xs from CLI so that it can download the binaries of those engines.

Install eshost-cli globally.

Run the following commands to configure the binaries of engines with eshost.

• eshost --add "v8" d8 $ESHOST_PATH_V8
• eshost --add "xs" xs $ESHOST_PATH_XS

Now you are good to go to run yarn test

[muhammadahmadasifbhatti]

Currently, I am working the following changes but planing them for the follow-up PRs.

• yarn test should run all the files in the /test folder.
• yarn test filename should run the test from that specific file.
• See any system level libraries that provide time in ~ns.

[erights]

LGTM, thanks!!!!

[muhammadahmadasifbhatti]

@erights I have addressed the comments. I think now I can tag @kriskowal to have a final look.
Nevertheless, I am done with another small feature that when you use this tool from CLI it can take filename as the input. That PR will be ready soon and you will be able to review that on Monday.

I also have Richard's idea in mind to have an executable like ava but before that I'll write a js script that'll have an entry point in the package.json.

[erights]

Hi @muhammadahmadasifbhatti , I resolved all the conversations above that are done, or that were "No change suggested" that you thumbs-uped. That turned out to be all of them ;). I also approved the workflows and see we still have CI errors. The lint error looks like the one that would be fixed by doing a yarn format in the top level endo directory.

[muhammadahmadasifbhatti]

Hi @muhammadahmadasifbhatti , I resolved all the conversations above that are done, or that were "No change suggested" that you thumbs-uped. That turned out to be all of them ;). I also approved the workflows and see we still have CI errors. The lint error looks like the one that would be fixed by doing a yarn format in the top level endo directory.

Thanks!
I just ran the yarn format command and pushed the changes. Hopefully, CI passes this time.

[erights]

In the last CI run, it looks like you were also getting

command not found: eshost

errors. I'm not sure what you're supposed to do so this is available in CI. @gibson042 do you know? Since (I think) we're already using eshost for other tests, why wasn't it already available?

@kriskowal , what permission is @muhammadahmadasifbhatti missing that needed me to approve the workflow for CI to run? Whatever that is, could you add it? Thanks.

[muhammadahmadasifbhatti]

In the last CI run, it looks like you were also getting

command not found: eshost

errors. I'm not sure what you're supposed to do so this is available in CI. @gibson042 do you know? Since (I think) we're already using eshost for other tests, why wasn't it already available?

@kriskowal , what permission is @muhammadahmadasifbhatti missing that needed me to approve the workflow for CI to run? Whatever that is, could you add it? Thanks.

For the above mentioned error, I think we might need to update the yaml file of the workflow to install eshost globally if that is not case already.

I am also getting another error which seems to be a linting error.

This expression is not callable.
  Type 'typeof import("/Users/muhammadahmad/Desktop/agoric-repos/endo/packages/benchmark/node_modules/@rollup/plugin-commonjs/types/index")' has no call signatures

VSCode is also squiggly here but the code is executing perfectly in both the engines. I will post the update in case I find anything.

[erights]

@michaelfig @gibson042 , do you agree that the lint complaint about the await on this line 7 is incorrect and a bug in our lint rule, since the earlier top-level await null; on line 4 should have closed the synchronous prelude?

[erights]

Huh. Or maybe the "no await inside a loop" is a pre-existing lint setting separate from any of our await safety rules? In which case we should turn off this pre-existing lint rule globally, i.e., everywhere our own await safety rules are enforced? Attn @kriskowal @mhofman @turadg

[turadg]

https://eslint.org/docs/latest/rules/no-await-in-loop is built into Eslint. It's not for safety so I don't think the Jessie rule affects whether to enable it.

btw, I don't see any linting output in CI because of this error:

Run yarn lint
Checking formatting...
All matched files use Prettier code style!
Error: rollup.config.js(12,13): error TS23[4](https://github.com/endojs/endo/actions/runs/13358283307/job/37304093134#step:7:5)9: This expression is not callable.
  Type 'typeof import("/home/runner/work/endo/endo/packages/benchmark/node_modules/@rollup/plugin-node-resolve/types/index")' has no call signatures.

[muhammadahmadasifbhatti]

I am of this opinion that we should disable no-await-in-loop rule for this line. Reason being, we actually need to check the performance of the function execution separately.

@turadg the issue you mentioned in the code snippet above is being resolved but the workflow has not been ran after the update.

[michaelfig]

@turadg writes:

It's not for safety so I don't think the Jessie rule affects whether to enable it.

Correct. I consider it a reminder for folks to use for-await-of instead if that makes the code more straightforward.

@muhammadahmadasifbhatti writes:

I am of this opinion that we should disable no-await-in-loop rule for this line.

I agree with that opinion.

[muhammadahmadasifbhatti]

test job is failing and the reason is: eshost not found. I have some solutions in mind

• Update the .github/workflows/ci.yml and install eshost globally.
• Install eshost in the devDependencies and then use yarn eshost.
• Just use yarn dlx as it will temporarily installs and runs a package.

Calling out the expert opinion. I would go for the 1st one as it is the simplest solution. Also, we might be needing eshost in other yarn scripts so, it is better to install it globally.

[turadg]

we might be needing eshost in other yarn scripts

True, but that's not a good reason to install it globally. The downsides of a global install is isn't automatically in all environments and it may be the wrong version. This is why we have package managers 😀

I suggest a fourth option:

• 'devDependencies' so it installs a particular version AND "npm exec eshost" so it runs independently of the package manager