-
-
Notifications
You must be signed in to change notification settings - Fork 419
Add node:sqlite benchmark #1334
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: master
Are you sure you want to change the base?
Conversation
@@ -69,7 +69,7 @@ for (const trial of trials) { | |||
const ctx = createContext(trial, driver); | |||
process.stdout.write(`${driver} (running...)\n`); | |||
try { | |||
const result = execFileSync('node', ['./benchmark.js', ctx], { stdio: 'pipe', encoding: 'utf8' }); | |||
const result = execFileSync('node', [...process.execArgv, './benchmark.js', ctx], { stdio: 'pipe', encoding: 'utf8' }); |
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.
What's the purpose of ...process.execArgv
?
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.
If you don't pass --experimental-sqlite
to node, the node:sqlite
module will not be available. This makes it so node child processes get the same node-specific arguments as the parent.
const sql = `SELECT ${columns.join(', ')} FROM ${table} WHERE rowid >= ? LIMIT 100`; | ||
let rowid = -100; | ||
return () => { | ||
const stmt = db.prepare(sql); |
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.
The SQLITE_PREPARE_PERSISTENT
flag is just an optimization hint. You should be able to reuse prepared statements without it.
|
||
return () => { | ||
// Error: statement has been finalized | ||
// for (const row of db.prepare(sql).iterate((rowid += 100) % count + 1)) {} |
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'll probably wait to merge this until node:sqlite
is a little more stable. But thanks for putting this together!
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.
When I made this there were a few issues with node:sqlite
. You can try to rerun it today against latest node and see if they are fixed or adjust the code if the functions are different. I think it is still better than nothing so you can at least get an idea of how the two implementations perform, even if this test does not work.
This adds a benchmark for the built-in node:sqlite module available with
--experimental-sqlite
since Node v22.5.0.This benchmark will only run and be included in the results if the
node:sqlite
module exists and is available.Known issues:
sqlite3_prepare_v2
that doesn't acceptSQLITE_PREPARE_PERSISTENT
flag.This is probably OK as a first pass and will be more useful later as the built-in module gets more stable.
Fixes #1266.