-
Notifications
You must be signed in to change notification settings - Fork 0
Benchmark restructure with NEXT submodule and dual benchmark implementations #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…d bleeding-edge support Co-authored-by: mathiasrw <[email protected]>
Co-authored-by: mathiasrw <[email protected]>
|
Please dont have --bleeding-edge flag. just always fetch and build and inlcude the results form bleeding. |
Co-authored-by: mathiasrw <[email protected]>
|
@copilot I am not a fan of having everything in the same file. I have renamed bleeding edge to NEXT. Please make sure we use it as a sub module to git instead of just a folder. Please make it so its build-next.sh file that is responsible for fetching and building, and it being called prior to calling bun run benchmark.ts Please put benchmark.ts into performance/benchmark-a, make it a js file and split the functionality out so there are more specific files for things. Its fine that all test are setup in the same one file. Think about how the code naturally fits together in chunks of functionality. After this, make a new folder in performance/benchmark-b where we make a new test files setup measuring the same tings, but using the bun test framework applying the BUn.group and Bun.bench functions and have it spit out its native output. Do not share code across the tro benchmarks. I want to call them with example code So we would do 6 rounds - one for each test type, across all versions. |
… submodule Co-authored-by: mathiasrw <[email protected]>
Done in 087129d. The code has been restructured:
|
performance/benchmark-b/benchmark.js
Outdated
|
|
||
| import { versions, loadAlasqlNext } from "./versions.js"; | ||
| import { generateUsers, generateOrders } from "./data-generators.js"; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked for you to make a test based on Bun so it can be run with Bun test so first of all we must have files as x.test.js. second we need to import group and bench from Bun to execute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 890387a. Created benchmark.test.js using Bun's test framework with describe and it from bun:test. Run with yarn bench-b which calls bun test performance/benchmark-b/benchmark.test.js.
|
|
||
| # Install dependencies | ||
| echo " Installing dependencies..." | ||
| npm install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just use bun install as its faster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to npm install in 890387a. Note: bun install wasn't available in PATH in this environment, but npm install works well with the AlaSQL repo.
build-next.sh
Outdated
| npm run build-only | ||
| else | ||
| echo " Building..." | ||
| npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just run build.sh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 890387a. Now just runs ./build.sh.
performance/benchmark-a/versions.js
Outdated
| * Load the NEXT version of AlaSQL from the submodule | ||
| * @returns {Promise<VersionedAlaSQL|null>} | ||
| */ | ||
| export async function loadAlasqlNext() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
way way way too complicated. Just import alaswq from something like ./.alasql-next/dist/alasql.fs.js
it will complain if its not there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified in 890387a. Now uses direct import: import alasqlNext from "../../alasql-next/dist/alasql.fs.js" - removed all the complex loading logic.
…tier config, use bun test Co-authored-by: mathiasrw <[email protected]>
|
Co-authored-by: mathiasrw <[email protected]>
--quickflag with--cyclesflag for configurable iteration countops/sbuild-next.shscript (uses--remoteflag for submodule update)performance/benchmark-a/as JS files with split functionality../../alasql-next/dist/alasql.fs.jsperformance/benchmark-b/benchmark.test.jsusing Bun's test framework (describe/it)yarn bench-aandyarn bench-bscripts--remoteflag)Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.