feat(bench-test): Benchmark Testing #2720

muhammadahmadasifbhatti · 2025-02-13T07:34:45Z

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 · 2025-02-13T07:55:34Z

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.

packages/benchmark/src/benchmark.js

packages/benchmark/README.md

erights

LGTM, thanks!!!!

packages/benchmark/CHANGELOG.md

packages/benchmark/CPU.20250213.123759.95202.0.001.cpuprofile

packages/benchmark/src/benchmark.js

packages/benchmark/test/index.test.js

packages/benchmark/package.json

muhammadahmadasifbhatti · 2025-02-14T18:47:44Z

@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 · 2025-02-16T19:46:25Z

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 · 2025-02-16T19:54:44Z

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 · 2025-02-16T19:58:57Z

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 · 2025-02-16T20:05:35Z

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 · 2025-02-17T02:42:50Z

packages/benchmark/src/benchmark.js

+  await null;
+  const start = performance.now();
+  for (let i = 0; i < iterations; i += 1) {
+    await fn();


@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?

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

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.

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.

@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 · 2025-02-17T18:53:18Z

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 · 2025-02-17T23:12:58Z

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

muhammadahmadasifbhatti · 2025-02-19T10:20:57Z

Fixed the CI.
Another quick follow-up PR is coming where I add the --verbose flag. Right now, the output is verbose by default and there is a bunch of info(output) that a developer does not want to see. By adding the flag, I will make it optional.

gibson042 · 2025-02-19T14:16:56Z

FWIW, I consider dependency upon eshost to be inappropriate here... it makes sense for esbench, but what this application really needs is an entry point configured with a collection of generic commands which it can invoke with a path to an input file—and for Agoric's purposes, the command for XS should wrap xsnap rather than xst and the command for V8 should just be node (maybe with options for self-identification as a benchmark subprocess).

But we're still iterating, and if it is delivering utility with eshost then full speed ahead and don't sweat the details.

gibson042 · 2025-02-19T14:21:12Z

packages/benchmark/src/benchmark.js

@@ -0,0 +1,39 @@
+const performance = { now: () => Date.now() * 1_000_000 };


This is not representative of performance.now, which returns milliseconds.

Suggested change

const performance = { now: () => Date.now() * 1_000_000 };

const performance = { now: () => Date.now() };

Umm you are right. The name and function are conflicting.
We just want to measure the time in nano-seconds. So, if anything's gonna change it's the name of the function.

Please, do post any suggestions related to precise time measurement.

Changed the name.

muhammadahmadasifbhatti · 2025-02-19T19:09:48Z

FWIW, I consider dependency upon eshost to be inappropriate here... it makes sense for esbench, but what this application really needs is an entry point configured with a collection of generic commands which it can invoke with a path to an input file—and for Agoric's purposes, the command for XS should wrap xsnap rather than xst and the command for V8 should just be node (maybe with options for self-identification as a benchmark subprocess).

But we're still iterating, and if it is delivering utility with eshost then full speed ahead and don't sweat the details.

@gibson042 thanks for the input. I've noted your opinion and added it in the follow-up list.
As you mentioned, and also, what I have in my mind; we are inclined towards ship and iterate. Current focus is to provide the bare minimum usability. The changes you suggested might be in the next PR.

muhammadahmadasifbhatti · 2025-02-21T15:58:31Z

@gibson042 just leaving a reminder to have a final look at the PR.

muhammadahmadasifbhatti · 2025-02-24T06:17:13Z

I have created all the follow-up tickets on the SRE board. Here is the link to them.

@gibson042 please feel free to leave more suggestions and feedbacks. I would highly appreciate if you can review it today.

gibson042

LGTM after addressing a few notes.

[also, capturing comments from elsewhere (without them blocking this PR)]

I think we want to drop the nesting; there's no need for a test function.

import { benchmark } from '@endo/benchmark';

// Each benchmark will be run with each element of the cross product of all arguments:
// * length 10, fillWith 'x'
// * length 100, fillWith 'x'
// * length 1000, fillWith 'x'
//
// This supports investigation of both scaling properties and of asymmetric input
// (cache hit vs. miss, match vs. not, accept vs. reject, small vs. large input, etc.).
// But such configuration is optional; when absent, each benchmark is observed without variations.
benchmark.setArgumentDomains({
  length: [10, 100, 1000],
  fillWith: ['x'],
});

// This assumes that values from `setArgumentDomains` are provided as
// separate arguments. Another alternative is providing a single record with
// named properties, albeit with the minor extra costs and boilerplate
// associated with destructuring (e.g., `(b, { length, fillWith }) => {…}`).
benchmark('Array(length).fill', (b, length, fillWith) => {
  // To support measuring fast code that would be distorted by otherwise
  // unnecessary function calls, `N` tells the function under test how
  // many times to exercise itself. Each callback body should therefore
  // have this for-loop shape, and the framework might even detect
  // incorrect callbacks by initializing `N` to a self-replacing getter
  // (e.g., `{ gotN = true; defineProperty(b, 'N', { value: N }); return N; }`).
  for (let i = b.N; i >= 0; i--) {
    b.result = Array(length).fill(fillWith);
  }
});

// `benchmark.withTransform` is *roughly* analogous to ava `test.macro`.
// Transformation functionality is important for e.g. measuring
// `passStyleOf(bigObject)` without inappropriately including the time to
// construct `bigObject`.
const fromFiller = benchmark.withTransform((b, length, fillWith) => {
  // This transformation of length → { length } and fillWith → () => fillWith
  // takes place outside of measurement, and therefore avoids distorting it.
  return [{ length }, () => fillWith];
});
benchmark('Array.from', fromFiller, (b, lengthObj, fillFn) => {
  for (let i = b.N; i >= 0; i--) {
    b.result = Array.from(lengthObj, fillFn);
  }
});

// or with the record approach:
// ```js
// const fromFiller = benchmark.withTransform((b, { length, fillWith }) => {
//   return { lengthObj: { length }, fillFn: () => fillWith };
// });
// benchmark('Array.from', fromFiller, (b, { lengthObj, fillFn }) => {…});
// ```

// or further dropping the "with" formal pattern:
// ```js
// const fromFiller = (b, { length, fillWith }) => {
//   b.context = { lengthObj: { length }, fillFn: () => fillWith };
// };
// benchmark('Array.from', fromFiller, b => {
//   const { lengthObj, fillFn } = b.context;
//   for (let i = b.N; i >= 0; i--) {
//     b.result = Array.from(lengthObj, fillFn);
//   }
// });
// ```

gibson042 · 2025-02-24T14:15:54Z

packages/benchmark/README.md

+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`


Installing eshost-cli does not create these variables AFAIK; they're just suggestions from its README.

Suggested change

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`

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

* `eshost --add "v8" d8 "$HOME/.esvu/bin/v8"`

* `eshost --add "xs" xs "$HOME/.esvu/bin/xs"`

I copied from the eshost with the same motivation that sometimes people change the directories manually. So, instead of a hardcode path just using a variable.
But, I get the idea. I think it should be more straight forward. Changing it.

gibson042 · 2025-02-24T14:51:49Z

packages/benchmark/README.md

@@ -0,0 +1,17 @@
+# `@endo/benchmark`
+
+This package just provides just minimalistic ava-like interface to run benchmark tests. 


Suggested change

This package just provides just minimalistic ava-like interface to run benchmark tests.

This package just provides a minimalistic ava-like interface to run benchmark tests.

gibson042 · 2025-02-24T15:32:30Z

packages/benchmark/package.json

+    "postpack": "git clean -f '*.d.ts*'",
+    "prepack": "tsc --build tsconfig.build.json",
+    "test": "./run-tests.sh",
+    "bench": "yarn rollup -c && node --jitless --cpu-prof dist/bundle.js"


I find it very strange for yarn bench to run node --cpu-prof against the hard-coded test/index.test.js file.

Suggested change

"bench": "yarn rollup -c && node --jitless --cpu-prof dist/bundle.js"

"cpu-prof-test": "yarn rollup -c && node --jitless --cpu-prof dist/bundle.js"

gibson042 · 2025-02-24T15:33:58Z

packages/benchmark/run-tests.sh

+
+set -e 
+
+echo $(yarn --version)


Suggested change

echo $(yarn --version)

echo "yarn version: $(yarn --version)"

gibson042 · 2025-02-24T15:44:57Z

packages/benchmark/run-tests.sh

+    echo "Installing engines..."
+    INSTALL_OUTPU_XS=$(yarn dlx esvu install xs 2>&1) || INSTALL_STATUS_XS=$?
+    INSTALL_OUTPU_V8=$(yarn dlx esvu install v8 2>&1) || INSTALL_STATUS_V8=$?


I'm not comfortable with yarn test modifying $HOME.

Suggested change

echo "Installing engines..."

INSTALL_OUTPU_XS=$(yarn dlx esvu install xs 2>&1) || INSTALL_STATUS_XS=$?

INSTALL_OUTPU_V8=$(yarn dlx esvu install v8 2>&1) || INSTALL_STATUS_V8=$?

echo "xs and/or v8 not found in $HOME/.esvu/engines; please esvu install them."

# The bash "command not found" exit status code is 127.

# https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html#Exit-Status-1

exit 127

I get the idea but what we are gonna do for the CI? As per your suggestion, we need to update the ci.yaml to install esvu and then install the engines using esvu. But, for this particular PR, I am not comfortable with changing the ci.yaml file. Open for the suggestions.

Nevertheless, it's gonna change in the next couple of PRs when we will be using node and xsnap to run these tests.

Changing ci.yaml is entirely appropriate; this adds a new requirement on the host environment just like the one currently fulfilled by setup-node.

And for local purposes, a package.json script for use like yarn run install-engines would make the consequences more clear.

I am still having second thoughts about this change. Reason being, there are follow-up tasks aligned and this code might not even exist in the next PR. Also, if we make this change we have to reverse the ci.yaml?

Another reason is ci.yaml is being run for every package in the package/ directory. So, it will be an unnecessary side effects for other packages to install esvu and eshost globally.

@turadg had a similar idea in his mind to not install esvu and eshost globally. Link to the comment.

I am more inclined towards having different script (as you mentioned in the second comment).

And for local purposes, a package.json script for use like yarn run install-engines would make the consequences more clear.

yarn install-engines which will do all the esvu and eshost related stuff.

yarn test:bench to run the benchmark tests. In this case bundle the code and run the tests in index.test.js

yarn test to run yarn install-engines and yarn test:bench. Might be vague in names but open for suggestions.

But, even with different scripts yarn will be modifying the $HOME directory.

I am still having second thoughts about this change. Although, what you are saying is right, but we know that there are follow-up tasks aligned. This code might not even exist in the next PR. So, we have to reverse the ci.yaml again?

Yes. You can add a comment as a reminder.

Another reason is ci.yaml is being run for every package in the package/ directory. So, it will be unnecessary side effects to install esvu and eshost globally.

That's not what I'm seeing at https://github.com/endojs/endo/blob/master/.github/workflows/ci.yml. Only the job "test" runs yarn test at package scope, and it does so after some setup steps. So what I'm asking for is insertion of esvu engine installation and eshost setup before its yarn build step, which will run exactly as often as the rest of those steps (i.e., once for each item of the (node-version: [18.x, 20.x, 22.4.1], platform: [ubuntu-latest]) matrix cross product).

I am more inclined towards having different script (as you mentioned in the second comment).

Having the separate package.json script is good, and the CI job step could even use it to reduce duplication. But the step on its own does not resolve this issue, because CI must have the engines but local yarn test must not install them.

And for local purposes, a package.json script for use like yarn run install-engines would make the consequences more clear.

yarn install-engines which will do all the esvu and eshost related stuff.

yarn test:bench to run the benchmark tests. In this case bundle the code and run the tests in index.test.js

yarn test to run yarn install-engines and yarn test:bench. Might be vague in names but open for suggestions.

No, I'm still objecting to yarn test modifying $HOME.

gibson042 · 2025-02-24T15:46:54Z

packages/benchmark/run-tests.sh

+if are_engines_installed; then
+    echo "Engines already installed. Skipping installation."
+else
+    echo "Installing engines..."
+    INSTALL_OUTPU_XS=$(yarn dlx esvu install xs 2>&1) || INSTALL_STATUS_XS=$?
+    INSTALL_OUTPU_V8=$(yarn dlx esvu install v8 2>&1) || INSTALL_STATUS_V8=$?
+fi
+
+if [ -n "$INSTALL_STATUS_XS" ] || [ -n "$INSTALL_STATUS_V8" ]; then 
+    if are_engines_installed; then
+        echo "Engines installed successfully despite esvu error."
+    else
+        echo "Error installing XS or V8:"
+        echo "$INSTALL_OUTPU_XS"
+        echo "$INSTALL_OUTPU_V8"
+        exit 1
+    fi
+fi


I'm not comfortable with yarn test modifying $HOME.

Suggested change

if are_engines_installed; then

echo "Engines already installed. Skipping installation."

else

echo "Installing engines..."

INSTALL_OUTPU_XS=$(yarn dlx esvu install xs 2>&1) || INSTALL_STATUS_XS=$?

INSTALL_OUTPU_V8=$(yarn dlx esvu install v8 2>&1) || INSTALL_STATUS_V8=$?

fi

if [ -n "$INSTALL_STATUS_XS" ] || [ -n "$INSTALL_STATUS_V8" ]; then

if are_engines_installed; then

echo "Engines installed successfully despite esvu error."

else

echo "Error installing XS or V8:"

echo "$INSTALL_OUTPU_XS"

echo "$INSTALL_OUTPU_V8"

exit 1

fi

fi

if are_engines_installed; then

echo "Engines already installed. Skipping installation."

else

echo "xs and/or v8 not found in $HOME/.esvu/engines; please esvu install them."

# The bash "command not found" exit status code is 127.

# https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html#Exit-Status-1

exit 127

fi

Addressed above.

gibson042 · 2025-02-24T15:48:43Z

packages/benchmark/run-tests.sh

+ls -la "$HOME/.esvu/engines"
+ls -la "$HOME/.esvu/engines/xs"
+ls -la "$HOME/.esvu/engines/v8"
+
+
+chmod +x "$HOME/.esvu/engines/xs/xst"
+chmod +x "$HOME/.esvu/engines/v8/d8"
+


What purpose is served by these commands?

Suggested change

ls -la "$HOME/.esvu/engines"

ls -la "$HOME/.esvu/engines/xs"

ls -la "$HOME/.esvu/engines/v8"

chmod +x "$HOME/.esvu/engines/xs/xst"

chmod +x "$HOME/.esvu/engines/v8/d8"

The first three are the logs for the debugging purposes. I still wanted to have them in case something goes off after merging the PR in the CI.

In the last two lines, we are changing the permissions of engine binaries so that we can execute them to run our bundled.

The first three are the logs for the debugging purposes. I still wanted to have them in case something goes off after merging the PR in the CI.

But they won't serve that purpose without either a preceding echo or a set -x.

In the last two lines, we are changing the permissions of engine binaries so that we can execute them to run our bundled.

Aren't they already marked executable when installed? I've never had to chmod them locally.

But they won't serve that purpose without either a preceding echo or a set -x

I am able to see the logs both on local and on ci. Nevertheless, pushing an echo because that seems right.

Aren't they already marked executable when installed? I've never had to chmod them locally.

Nope, I had to change the mod locally. Also, at first I pushed the changes without changing the mod and got an error on the CI i.e. Either the file does not exist or it is not executable.

gibson042 · 2025-02-24T15:58:05Z

packages/benchmark/run-tests.sh

+yarn eshost --add "xs" xs "$HOME/.esvu/engines/xs/xst"
+yarn eshost --add "v8" d8 "$HOME/.esvu/engines/v8/d8"
+
+yarn eshost --list


Suggested change

yarn eshost --add "xs" xs "$HOME/.esvu/engines/xs/xst"

yarn eshost --add "v8" d8 "$HOME/.esvu/engines/v8/d8"

yarn eshost --list

yarn eshost -h xs,v8 -se true >/dev/null 2>&1 || ESHOST_STATUS=$?

if [ "${ESHOST_STATUS:-0}" != 0 ]; then

echo "xs and/or v8 not known to eshost; please eshost --add them per README.md."

exit $ESHOST_STATUS

fi

Again, I did this for the CI.

But it shouldn't degrade the local experience. Something like this would still accomplish CI visibility:

yarn eshost -h xs,v8 -se true >/dev/null 2>&1 || ESHOST_STATUS=$? if [ "${ESHOST_STATUS:-0}" != 0 ]; then echo "xs and/or v8 not known to eshost; please eshost --add them per README.md." echo "# eshost --list" yarn eshost --list exit $ESHOST_STATUS fi

I think we are not on the same page here.
Let's suppose I accept the suggested changes. Now, the CI will run yarn test and it we will immediately faill because of error xs and/or v8 not known to eshost; please eshost --add them per README.md.

This will work only if we update the ci.yaml to run these commands (of adding the engines) for us.

This will work only if we update the ci.yaml to run these commands (of adding the engines) for us.

Yes, that's exactly what I'm requesting. yarn test on its own requires the engines to already exist and fails with an informative message when they don't. ci.yaml makes sure that the engines are installed before invoking yarn test.

gibson042 · 2025-02-24T15:58:30Z

packages/benchmark/run-tests.sh

+echo "Running eshost..."
+
+yarn rollup -c
+
+yarn eshost -h xs,v8 dist/bundle.js


Suggested change

echo "Running eshost..."

yarn rollup -c

yarn eshost -h xs,v8 dist/bundle.js

yarn rollup -c

yarn eshost -h xs,v8 dist/bundle.js

muhammadahmadasifbhatti · 2025-02-25T10:06:49Z

For this comment.
By looking at the code you suggested, I guess this will change the whole scope of the PR. What do you think? The idea is to ship and iterate and think this way we will be blocked on this another couple of days.

One more thing; Although, we both agree that benchmark should not be nested but we might need some other opinions on this. So, we can ask this question in the endo channel and then work on it.

gibson042 · 2025-02-25T14:53:10Z

For this comment. By looking at the code you suggested, I guess this will change the whole scope of the PR. What do you think? The idea is to ship and iterate and think this way we will be blocked on this another couple of days.

Right, I hoped the preface would make that clear. That comment is not requesting changes for this PR, just documenting more publicly where I want things to end up.

gibson042 · 2025-02-25T20:10:21Z

packages/benchmark/run-tests.sh

+echo $(ls -la "$HOME/.esvu/engines")
+echo $(ls -la "$HOME/.esvu/engines/xs")
+echo $(ls -la "$HOME/.esvu/engines/v8")
+
+
+chmod +x "$HOME/.esvu/engines/xs/xst"
+chmod +x "$HOME/.esvu/engines/v8/d8"
+
+yarn eshost --add "xs" xs "$HOME/.esvu/engines/xs/xst"
+yarn eshost --add "v8" d8 "$HOME/.esvu/engines/v8/d8"
+


None of this belongs outside of a "setup host environment" container.

Suggested change

echo $(ls -la "$HOME/.esvu/engines")

echo $(ls -la "$HOME/.esvu/engines/xs")

echo $(ls -la "$HOME/.esvu/engines/v8")

chmod +x "$HOME/.esvu/engines/xs/xst"

chmod +x "$HOME/.esvu/engines/v8/d8"

yarn eshost --add "xs" xs "$HOME/.esvu/engines/xs/xst"

yarn eshost --add "v8" d8 "$HOME/.esvu/engines/v8/d8"

gibson042 · 2025-02-25T20:14:48Z

packages/benchmark/run-tests.sh

+echo $(ls -la "$HOME/.esvu/engines")
+echo $(ls -la "$HOME/.esvu/engines/xs")
+echo $(ls -la "$HOME/.esvu/engines/v8")


I think you misunderstood my "echo" suggestion; the point was to indicate which directory is being listed.

Suggested change

echo $(ls -la "$HOME/.esvu/engines")

echo $(ls -la "$HOME/.esvu/engines/xs")

echo $(ls -la "$HOME/.esvu/engines/v8")

for d in "$HOME/.esvu/engines" "$HOME/.esvu/engines/xs" "$HOME/.esvu/engines/v8"; do

echo "# $d"

ls -la "$d"

done

init

b77963a

muhammadahmadasifbhatti mentioned this pull request Feb 13, 2025

fix(benchmark-test): suggestions for #10975 Agoric/agoric-sdk#10982

Closed

updated scripts: test and bench

5a5bc59

muhammadahmadasifbhatti marked this pull request as ready for review February 13, 2025 07:40

Muneeb147 reviewed Feb 13, 2025

View reviewed changes

packages/benchmark/src/benchmark.js Show resolved Hide resolved

Muneeb147 reviewed Feb 13, 2025

View reviewed changes

packages/benchmark/README.md Show resolved Hide resolved

added readme

ad54242

erights approved these changes Feb 14, 2025

View reviewed changes

addressed mark's comments

169e0fe

muhammadahmadasifbhatti changed the title ~~Benchmark Testing~~ feat(bench-test): Benchmark Testing Feb 14, 2025

Muhammad Ahmad and others added 2 commits February 16, 2025 23:22

ava not needed in yarn.lock

80a45c3

Merge branch 'master' into ahmad-benchmark-init

05469f9

format

85727ee

Removed commonjs. No need to bundle the code into es6

c98abcd

erights reviewed Feb 17, 2025

View reviewed changes

Muhammad Ahmad added 2 commits February 17, 2025 16:56

fix CI, added endo/init and endo/pass-style, removed void

7400948

format and lint

f8f53a0

Muhammad Ahmad added 3 commits February 18, 2025 00:01

testing: update CI to install eshost globally

58d56d1

use shell script

0586ba6

remove yarn add global eshost

a337498

muhammadahmadasifbhatti and others added 2 commits February 19, 2025 13:04

Merge branch 'master' into ahmad-benchmark-init

232acf6

fix:ci

4095f34

Muhammad Ahmad added 3 commits February 19, 2025 14:20

check the error

5ea5576

install engines separtely

6d4f68a

pass the test

e9c08da

check both status

aef3d55

Muneeb147 approved these changes Feb 19, 2025

View reviewed changes

gibson042 reviewed Feb 19, 2025

View reviewed changes

muhammadahmadasifbhatti requested a review from gibson042 February 19, 2025 19:13

Muhammad Ahmad added 2 commits February 20, 2025 00:30

fix(time): change the function name to get time

da0d44f

fix(camelCase): Change to

b5a9bf0

gibson042 approved these changes Feb 24, 2025

View reviewed changes

Addressed Richard's comments

286c377

fix: add echo to run-test.sh

0f9bd62

gibson042 requested changes Feb 25, 2025

View reviewed changes

		@@ -0,0 +1,39 @@
		const performance = { now: () => Date.now() * 1_000_000 };

		@@ -0,0 +1,17 @@
		# `@endo/benchmark`

		This package just provides just minimalistic ava-like interface to run benchmark tests.

	This package just provides just minimalistic ava-like interface to run benchmark tests.
	This package just provides a minimalistic ava-like interface to run benchmark tests.

	"bench": "yarn rollup -c && node --jitless --cpu-prof dist/bundle.js"
	"cpu-prof-test": "yarn rollup -c && node --jitless --cpu-prof dist/bundle.js"

	echo $(yarn --version)
	echo "yarn version: $(yarn --version)"

-    echo "Installing engines..."
-    INSTALL_OUTPU_XS=$(yarn dlx esvu install xs 2>&1) || INSTALL_STATUS_XS=$?
-    INSTALL_OUTPU_V8=$(yarn dlx esvu install v8 2>&1) || INSTALL_STATUS_V8=$?
+    echo "xs and/or v8 not found in $HOME/.esvu/engines; please esvu install them."
+    # The bash "command not found" exit status code is 127.
+    # https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html#Exit-Status-1
+    exit 127

-yarn eshost --add "xs" xs "$HOME/.esvu/engines/xs/xst"
-yarn eshost --add "v8" d8 "$HOME/.esvu/engines/v8/d8"
-yarn eshost --list
+yarn eshost -h xs,v8 -se true >/dev/null 2>&1 || ESHOST_STATUS=$?
+if [ "${ESHOST_STATUS:-0}" != 0 ]; then
+    echo "xs and/or v8 not known to eshost; please eshost --add them per README.md."
+    exit $ESHOST_STATUS
+fi

feat(bench-test): Benchmark Testing #2720

Are you sure you want to change the base?

feat(bench-test): Benchmark Testing #2720

Conversation

muhammadahmadasifbhatti commented Feb 13, 2025

Description

How to run locally

Pre-reqs of running yarn test

Install esvu globally

Install eshost-cli globally.

muhammadahmadasifbhatti commented Feb 13, 2025

erights left a comment

Choose a reason for hiding this comment

muhammadahmadasifbhatti commented Feb 14, 2025

erights commented Feb 16, 2025

muhammadahmadasifbhatti commented Feb 16, 2025

erights commented Feb 16, 2025

muhammadahmadasifbhatti commented Feb 16, 2025

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

muhammadahmadasifbhatti commented Feb 17, 2025 • edited Loading

turadg commented Feb 17, 2025 • edited Loading

muhammadahmadasifbhatti commented Feb 19, 2025

gibson042 commented Feb 19, 2025

Choose a reason for hiding this comment

muhammadahmadasifbhatti Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhammadahmadasifbhatti commented Feb 19, 2025

muhammadahmadasifbhatti commented Feb 21, 2025

muhammadahmadasifbhatti commented Feb 24, 2025

gibson042 left a comment • 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

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

muhammadahmadasifbhatti Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 Feb 25, 2025 • 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

muhammadahmadasifbhatti Feb 25, 2025 • 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

Choose a reason for hiding this comment

muhammadahmadasifbhatti commented Feb 25, 2025

gibson042 commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pre-reqs of running `yarn test`

Install `esvu` globally

Install `eshost-cli` globally.

muhammadahmadasifbhatti commented Feb 17, 2025 •

edited

Loading

turadg commented Feb 17, 2025 •

edited

Loading

muhammadahmadasifbhatti Feb 19, 2025 •

edited

Loading

gibson042 left a comment •

edited

Loading

muhammadahmadasifbhatti Feb 25, 2025 •

edited

Loading

gibson042 Feb 25, 2025 •

edited

Loading

muhammadahmadasifbhatti Feb 25, 2025 •

edited

Loading