Skip to content

Commit 68df1db

Browse files
authored
sqlite: use shared-shape objects for result rows
`all()`, `get()` and `iterate()` built each row with the `Object::New()` overload that takes names and values. That overload always returns a dictionary-mode object, so no two rows shared a map and every property read was a hash lookup. Build rows from a `DictionaryTemplate` cached on the statement in place of the column names, invalidated on re-prepare. Rows keep their null prototype. Statements whose column names a template cannot express (array indices, duplicates, non-ASCII names, which the template interns as Latin-1) or with more than 64 columns keep the previous path. Add a benchmark that reads every column of each row, since the existing ones only measure building rows. Fixes: #65799 Assisted-by: Claude Signed-off-by: Guilherme Araújo <arauujogui@gmail.com> PR-URL: #66385 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 496b48b commit 68df1db

4 files changed

Lines changed: 207 additions & 54 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
'use strict';
2+
const common = require('../common.js');
3+
const sqlite = require('node:sqlite');
4+
const assert = require('assert');
5+
6+
const bench = common.createBenchmark(main, {
7+
n: [1e4],
8+
rows: [1, 100],
9+
method: ['all', 'iterate', 'get'],
10+
returnArrays: [0, 1],
11+
});
12+
13+
function main(conf) {
14+
const db = new sqlite.Database(':memory:');
15+
db.exec(
16+
'CREATE TABLE foo (text_column TEXT, integer_column INTEGER, real_column REAL, blob_column BLOB)',
17+
);
18+
const insert = db.prepare('INSERT INTO foo VALUES (?, ?, ?, ?)');
19+
for (let i = 0; i < conf.rows; i++) {
20+
insert.run(`text ${i}`, i, i / 3, Buffer.from('example blob data'));
21+
}
22+
23+
const stmt = db.prepare(`SELECT * FROM foo LIMIT ${conf.rows}`);
24+
stmt.setReturnArrays(conf.returnArrays === 1);
25+
26+
// Each row is built and then has every column read once.
27+
const read = conf.returnArrays ?
28+
(row) => row[0].length + row[1] + row[2] + row[3].length :
29+
(row) => row.text_column.length + row.integer_column +
30+
row.real_column + row.blob_column.length;
31+
32+
let sum = 0;
33+
bench.start();
34+
for (let i = 0; i < conf.n; i++) {
35+
if (conf.method === 'get') {
36+
sum += read(stmt.get());
37+
} else {
38+
for (const row of stmt[conf.method]()) sum += read(row);
39+
}
40+
}
41+
bench.end(conf.n);
42+
43+
assert.ok(sum > 0);
44+
}

‎src/node_sqlite.cc‎

Lines changed: 79 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <cstring>
2323
#include <limits>
2424
#include <string>
25+
#include <string_view>
2526

2627
namespace node {
2728
namespace sqlite {
@@ -3812,6 +3813,7 @@ void Statement::Finalize() {
38123813

38133814
void Statement::InvalidateColumnNameCache() {
38143815
cached_column_names_.clear();
3816+
cached_row_template_.Reset();
38153817
cached_column_names_reprepare_count_ = -1;
38163818
}
38173819

@@ -4033,37 +4035,79 @@ MaybeLocal<Name> Statement::ColumnNameToName(const int column) {
40334035
.As<Name>();
40344036
}
40354037

4036-
// Populates `keys` with cached column names, rebuilding the cache if the
4037-
// statement was re-prepared.
4038-
bool Statement::GetCachedColumnNames(LocalVector<Name>* keys) {
4039-
Isolate* isolate = env()->isolate();
4038+
// Size of the stack array RowToObject passes to the template.
4039+
static constexpr int kMaxRowTemplateColumns = 64;
4040+
4041+
// Templates read names as Latin-1 and reject indices and duplicates.
4042+
static bool CanUseInRowTemplate(Local<Context> context,
4043+
Local<Name> key,
4044+
std::string_view name,
4045+
const std::vector<std::string_view>& seen) {
4046+
return simdutf::validate_ascii(name.data(), name.size()) &&
4047+
std::find(seen.begin(), seen.end(), name) == seen.end() &&
4048+
key->ToArrayIndex(context).IsEmpty();
4049+
}
40404050

4051+
// Fills either cached_row_template_ or cached_column_names_.
4052+
bool Statement::UpdateRowCache() {
40414053
const int reprepare_count =
40424054
sqlite3_stmt_status(statement_.get(), SQLITE_STMTSTATUS_REPREPARE, false);
4043-
if (reprepare_count != cached_column_names_reprepare_count_) {
4044-
cached_column_names_.clear();
4045-
const int num_cols = sqlite3_column_count(statement_.get());
4046-
if (num_cols == 0) {
4047-
cached_column_names_reprepare_count_ = reprepare_count;
4048-
return true;
4049-
}
4055+
if (reprepare_count == cached_column_names_reprepare_count_) return true;
4056+
4057+
InvalidateColumnNameCache();
4058+
Isolate* isolate = env()->isolate();
4059+
Local<Context> context = env()->context();
4060+
const int num_cols = sqlite3_column_count(statement_.get());
4061+
LocalVector<Name> keys(isolate);
4062+
std::vector<std::string_view> names;
4063+
keys.reserve(num_cols);
4064+
names.reserve(num_cols);
4065+
bool use_template = num_cols <= kMaxRowTemplateColumns;
4066+
for (int i = 0; i < num_cols; ++i) {
4067+
Local<Name> key;
4068+
if (!ColumnNameToName(i).ToLocal(&key)) return false;
4069+
std::string_view name = sqlite3_column_name(statement_.get(), i);
4070+
use_template =
4071+
use_template && CanUseInRowTemplate(context, key, name, names);
4072+
keys.push_back(key);
4073+
names.push_back(name);
4074+
}
4075+
4076+
if (use_template) {
4077+
cached_row_template_.Reset(
4078+
isolate,
4079+
DictionaryTemplate::New(isolate, {names.data(), names.size()}));
4080+
} else {
40504081
cached_column_names_.reserve(num_cols);
4051-
for (int i = 0; i < num_cols; ++i) {
4052-
Local<Name> key;
4053-
if (!ColumnNameToName(i).ToLocal(&key)) {
4054-
InvalidateColumnNameCache();
4055-
return false;
4056-
}
4057-
cached_column_names_.emplace_back(Global<Name>(isolate, key));
4082+
for (Local<Name> key : keys) {
4083+
cached_column_names_.emplace_back(isolate, key);
40584084
}
4059-
cached_column_names_reprepare_count_ = reprepare_count;
40604085
}
4086+
cached_column_names_reprepare_count_ = reprepare_count;
4087+
return true;
4088+
}
40614089

4062-
keys->reserve(cached_column_names_.size());
4063-
for (const auto& name : cached_column_names_) {
4064-
keys->emplace_back(name.Get(isolate));
4090+
MaybeLocal<Object> Statement::RowToObject(LocalVector<Value>* values) {
4091+
DCHECK_EQ(
4092+
sqlite3_stmt_status(statement_.get(), SQLITE_STMTSTATUS_REPREPARE, false),
4093+
cached_column_names_reprepare_count_);
4094+
Isolate* isolate = env()->isolate();
4095+
if (cached_row_template_.IsEmpty()) {
4096+
LocalVector<Name> keys(isolate);
4097+
keys.reserve(cached_column_names_.size());
4098+
for (const auto& name : cached_column_names_) {
4099+
keys.emplace_back(name.Get(isolate));
4100+
}
4101+
DCHECK_EQ(keys.size(), values->size());
4102+
return Object::New(
4103+
isolate, Null(isolate), keys.data(), values->data(), keys.size());
40654104
}
4066-
return true;
4105+
DCHECK_LE(values->size(), kMaxRowTemplateColumns);
4106+
MaybeLocal<Value> template_values[kMaxRowTemplateColumns];
4107+
std::copy(values->begin(), values->end(), template_values);
4108+
return NewDictionaryInstanceNullProto(env()->context(),
4109+
cached_row_template_.Get(isolate),
4110+
{template_values, values->size()});
40674111
}
40684112

40694113
MaybeLocal<Value> StatementExecutionHelper::ColumnToValue(Environment* env,
@@ -4108,12 +4152,14 @@ MaybeLocal<Value> StatementExecutionHelper::All(Environment* env,
41084152
int num_cols = 0;
41094153
LocalVector<Value> rows(isolate);
41104154
LocalVector<Value> row_values(isolate);
4111-
LocalVector<Name> row_keys(isolate);
41124155

41134156
SteppingStatementGuard stepping(db, stmt);
41144157
while ((r = sqlite3_step(stmt)) == SQLITE_ROW) {
41154158
if (num_cols == 0) {
41164159
num_cols = sqlite3_column_count(stmt);
4160+
if (!return_arrays && !statement->UpdateRowCache()) {
4161+
return MaybeLocal<Value>();
4162+
}
41174163
}
41184164

41194165
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
@@ -4126,16 +4172,10 @@ MaybeLocal<Value> StatementExecutionHelper::All(Environment* env,
41264172
Array::New(isolate, row_values.data(), row_values.size());
41274173
rows.emplace_back(row_array);
41284174
} else {
4129-
if (row_keys.size() == 0) {
4130-
// Reuses the statement's internalized column names instead of
4131-
// re-interning them on every call.
4132-
if (!statement->GetCachedColumnNames(&row_keys)) {
4133-
return MaybeLocal<Value>();
4134-
}
4175+
Local<Object> row_obj;
4176+
if (!statement->RowToObject(&row_values).ToLocal(&row_obj)) {
4177+
return MaybeLocal<Value>();
41354178
}
4136-
DCHECK_EQ(row_keys.size(), row_values.size());
4137-
Local<Object> row_obj = Object::New(
4138-
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
41394179
rows.emplace_back(row_obj);
41404180
}
41414181
}
@@ -4272,17 +4312,9 @@ MaybeLocal<Value> StatementExecutionHelper::Get(Environment* env,
42724312
Local<Value> result;
42734313
if (return_arrays) {
42744314
result = Array::New(isolate, row_values.data(), row_values.size());
4275-
} else {
4276-
LocalVector<Name> keys(isolate);
4277-
// Reuses the statement's internalized column names instead of
4278-
// re-interning them on every call.
4279-
if (!statement->GetCachedColumnNames(&keys)) {
4280-
return MaybeLocal<Value>();
4281-
}
4282-
4283-
DCHECK_EQ(keys.size(), row_values.size());
4284-
result = Object::New(
4285-
isolate, Null(isolate), keys.data(), row_values.data(), num_cols);
4315+
} else if (!statement->UpdateRowCache() ||
4316+
!statement->RowToObject(&row_values).ToLocal(&result)) {
4317+
return MaybeLocal<Value>();
42864318
}
42874319

42884320
RESET_AND_CHECK(isolate, db, stmt, needs_reset, MaybeLocal<Value>());
@@ -5083,7 +5115,6 @@ void StatementIterator::Next(const FunctionCallbackInfo<Value>& args) {
50835115

50845116
int num_cols = sqlite3_column_count(iter->stmt_->statement_.get());
50855117
Local<Value> row_value;
5086-
LocalVector<Name> row_keys(isolate);
50875118
LocalVector<Value> row_values(isolate);
50885119

50895120
if (ExtractRowValues(env,
@@ -5097,14 +5128,9 @@ void StatementIterator::Next(const FunctionCallbackInfo<Value>& args) {
50975128

50985129
if (iter->stmt_->return_arrays_) {
50995130
row_value = Array::New(isolate, row_values.data(), row_values.size());
5100-
} else {
5101-
// Use cached internalized column names to avoid repeated V8 string
5102-
// creation and enable hidden class sharing across row objects.
5103-
if (!iter->stmt_->GetCachedColumnNames(&row_keys)) return;
5104-
5105-
DCHECK_EQ(row_keys.size(), row_values.size());
5106-
row_value = Object::New(
5107-
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
5131+
} else if (!iter->stmt_->UpdateRowCache() ||
5132+
!iter->stmt_->RowToObject(&row_values).ToLocal(&row_value)) {
5133+
return;
51085134
}
51095135

51105136
MaybeLocal<Value> values[] = {Boolean::New(isolate, false), row_value};

‎src/node_sqlite.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,9 @@ class Statement : public BaseObject {
397397
static void Dispose(const v8::FunctionCallbackInfo<v8::Value>& args);
398398
v8::MaybeLocal<v8::Value> ColumnToValue(const int column);
399399
v8::MaybeLocal<v8::Name> ColumnNameToName(const int column);
400-
bool GetCachedColumnNames(v8::LocalVector<v8::Name>* keys);
400+
// Call after sqlite3_step(), which may re-prepare, and before RowToObject.
401+
bool UpdateRowCache();
402+
v8::MaybeLocal<v8::Object> RowToObject(v8::LocalVector<v8::Value>* values);
401403
void Finalize();
402404
bool IsFinalized();
403405

@@ -418,6 +420,8 @@ class Statement : public BaseObject {
418420
inline int ResetStatement();
419421
std::vector<v8::Global<v8::Name>> cached_column_names_;
420422
int cached_column_names_reprepare_count_ = -1;
423+
// Set instead of cached_column_names_ when the names fit a template.
424+
v8::Global<v8::DictionaryTemplate> cached_row_template_;
421425
void InvalidateColumnNameCache();
422426
bool BindParams(const v8::FunctionCallbackInfo<v8::Value>& args);
423427
bool BindValue(const v8::Local<v8::Value>& value, const int index);
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Flags: --allow-natives-syntax
2+
'use strict';
3+
const { skipIfSQLiteMissing } = require('../common');
4+
skipIfSQLiteMissing();
5+
const assert = require('node:assert');
6+
const { Database } = require('node:sqlite');
7+
const { suite, test } = require('node:test');
8+
9+
function assertSharedShape(rows) {
10+
for (const row of rows) {
11+
assert.strictEqual(Object.getPrototypeOf(row), null);
12+
assert(eval('%HasFastProperties(row)'));
13+
assert(eval('%HaveSameMap(row, rows[0])'));
14+
}
15+
}
16+
17+
suite('result row objects', () => {
18+
const db = new Database(':memory:');
19+
db.exec('CREATE TABLE t (a INTEGER, b TEXT, c REAL)');
20+
const insert = db.prepare('INSERT INTO t VALUES (?, ?, ?)');
21+
insert.run(1, 'x', 1.5);
22+
insert.run(2, 'y', 2.5);
23+
insert.run(3, null, null);
24+
25+
test('all() rows share a fast map', () => {
26+
const rows = db.prepare('SELECT * FROM t ORDER BY a').all();
27+
assert.deepStrictEqual(rows, [
28+
{ __proto__: null, a: 1, b: 'x', c: 1.5 },
29+
{ __proto__: null, a: 2, b: 'y', c: 2.5 },
30+
{ __proto__: null, a: 3, b: null, c: null },
31+
]);
32+
assertSharedShape(rows);
33+
});
34+
35+
test('get() rows share a fast map', () => {
36+
const stmt = db.prepare('SELECT * FROM t WHERE a = ?');
37+
assertSharedShape([stmt.get(1), stmt.get(2), stmt.get(3)]);
38+
});
39+
40+
test('iterate() rows share a fast map', () => {
41+
assertSharedShape(db.prepare('SELECT * FROM t ORDER BY a').iterate().toArray());
42+
});
43+
44+
test('a re-prepared statement picks up the new columns', () => {
45+
using db = new Database(':memory:');
46+
db.exec('CREATE TABLE s (a INTEGER)');
47+
db.exec('INSERT INTO s VALUES (1)');
48+
const stmt = db.prepare('SELECT * FROM s');
49+
assert.deepStrictEqual(stmt.get(), { __proto__: null, a: 1 });
50+
db.exec('ALTER TABLE s ADD COLUMN b INTEGER DEFAULT 2');
51+
assert.deepStrictEqual(stmt.get(), { __proto__: null, a: 1, b: 2 });
52+
});
53+
54+
test('array-index column names', () => {
55+
const stmt = db.prepare('SELECT 1 AS "0", 2 AS b');
56+
assert.deepStrictEqual(stmt.get(), { __proto__: null, 0: 1, b: 2 });
57+
assert.deepStrictEqual(stmt.all(), [{ __proto__: null, 0: 1, b: 2 }]);
58+
});
59+
60+
test('duplicate column names keep the last value', () => {
61+
const stmt = db.prepare('SELECT 1 AS a, 2 AS a');
62+
assert.deepStrictEqual(stmt.get(), { __proto__: null, a: 2 });
63+
assert.deepStrictEqual(stmt.iterate().toArray(),
64+
[{ __proto__: null, a: 2 }]);
65+
});
66+
67+
test('non-ASCII column names', () => {
68+
const row = db.prepare('SELECT 1 AS "café", 2 AS "名前"').get();
69+
assert.deepStrictEqual(row, { __proto__: null, café: 1, 名前: 2 });
70+
});
71+
72+
test('rows wider than the template limit', () => {
73+
const cols = Array.from({ length: 100 }, (_, i) => `${i} AS c${i}`);
74+
const row = db.prepare(`SELECT ${cols.join(', ')}`).get();
75+
const expected = { __proto__: null };
76+
for (let i = 0; i < 100; i++) expected[`c${i}`] = i;
77+
assert.deepStrictEqual(row, expected);
78+
});
79+
});

0 commit comments

Comments
 (0)