diff --git a/src/objects/statement.cpp b/src/objects/statement.cpp index 860c187..23aaa94 100644 --- a/src/objects/statement.cpp +++ b/src/objects/statement.cpp @@ -53,15 +53,15 @@ Statement::Extras::Extras(sqlite3_uint64 id) : bind_map(0), id(id) {} #if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 -RowBuilder Statement::NewRowBuilder(v8::Isolate* isolate, bool safe_ints) { - return RowBuilder(isolate, handle, safe_ints, &extras->column_keys); +RowBuilder Statement::NewRowBuilder(v8::Isolate* isolate, v8::Local ctx, bool safe_ints) { + return RowBuilder(isolate, ctx, handle, safe_ints, &extras->column_keys); } #endif v8::Local Statement::GetRowJS(v8::Isolate* isolate, v8::Local ctx, bool safe_ints, char mode) { #if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 if (mode == Data::FLAT) { - return NewRowBuilder(isolate, safe_ints).GetRowJS(); + return NewRowBuilder(isolate, ctx, safe_ints).GetRowJS(); } #endif return Data::GetRowJS(isolate, ctx, handle, safe_ints, mode); @@ -221,7 +221,7 @@ NODE_METHOD(Statement::JS_all) { rows.reserve(8); if (mode == Data::FLAT) { - RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, safe_ints); + RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, ctx, safe_ints); while (sqlite3_step(handle) == SQLITE_ROW) { rows.emplace_back(rowBuilder.GetRowJS()); } @@ -300,7 +300,7 @@ NODE_METHOD(Statement::JS_explainQueryPlan) { rows.reserve(8); if (mode == Data::FLAT) { - RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, safe_ints); + RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, ctx, safe_ints); while (sqlite3_step(handle) == SQLITE_ROW) { rows.emplace_back(rowBuilder.GetRowJS()); } diff --git a/src/objects/statement.hpp b/src/objects/statement.hpp index db6417a..56dd3b5 100644 --- a/src/objects/statement.hpp +++ b/src/objects/statement.hpp @@ -16,8 +16,8 @@ class Statement : public node::ObjectWrap { friend class StatementIterator; #if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 // Returns a builder of object rows that reuses the Statement's cached - // column names. - RowBuilder NewRowBuilder(v8::Isolate* isolate, bool safe_ints); + // column names and template. + RowBuilder NewRowBuilder(v8::Isolate* isolate, v8::Local ctx, bool safe_ints); #endif // Converts the current row to JavaScript, reusing the cached column names diff --git a/src/util/column-keys.cpp b/src/util/column-keys.cpp index bc4907d..e1e377c 100644 --- a/src/util/column-keys.cpp +++ b/src/util/column-keys.cpp @@ -1,11 +1,19 @@ -// The column names of a statement as internalized strings, cached across calls -// so that building object rows doesn't re-internalize them for every row. -// Names only change when SQLite re-prepares the statement (e.g. after a schema -// change behind `SELECT *`), so the cache is refreshed when the re-prepare -// count or the column count changes. +// The column names of a statement, cached across calls so that building object +// rows doesn't re-internalize them for every row. Names only change when SQLite +// re-prepares the statement (e.g. after a schema change behind `SELECT *`), so +// the cache is refreshed when the re-prepare count or the column count changes. +// +// When it can, it also caches a v8::DictionaryTemplate for the names. Objects +// made from it have fast properties and share a hidden class, while +// v8::Object::New() makes dictionary-mode objects, which are slower to create +// and to read. Creating the template costs about as much as building 10-20 +// rows with v8::Object::New(), so it is only created once the statement has +// built kRowsBeforeTemplate object rows. class ColumnKeys { public: + static constexpr int kRowsBeforeTemplate = 16; + explicit ColumnKeys() : reprepares(-1) {} // Must be called after sqlite3_step(), which is where re-preparing happens. @@ -13,13 +21,7 @@ class ColumnKeys { const int count = sqlite3_column_count(handle); const int current = sqlite3_stmt_status(handle, SQLITE_STMTSTATUS_REPREPARE, 0); if (current != reprepares || static_cast(keys.size()) != count) { - keys.clear(); - keys.reserve(count); - for (int i = 0; i < count; ++i) { - keys.emplace_back(isolate, - InternalizedFromUtf8(isolate, sqlite3_column_name(handle, i), -1).As() - ); - } + Refresh(isolate, handle, count); reprepares = current; } out.clear(); @@ -29,7 +31,77 @@ class ColumnKeys { } } + // Called for each object row built with the names last returned by Get(). + // Returns the template for those names, creating it once enough rows have + // been built. Returns an empty handle before then, or if the names can't + // be used for a template. + v8::Local TemplateForRow(v8::Isolate* isolate) { + if (dictionary_template.IsEmpty() && !names.empty() && ++rows >= kRowsBeforeTemplate) { + std::vector views(names.begin(), names.end()); + dictionary_template.Reset(isolate, v8::DictionaryTemplate::New(isolate, + v8::MemorySpan(views.data(), views.size()) + )); + names.clear(); + } + return dictionary_template.Get(isolate); + } + private: + + void Refresh(v8::Isolate* isolate, sqlite3_stmt* handle, int count) { + keys.clear(); + keys.reserve(count); + names.clear(); + names.reserve(count); + for (int i = 0; i < count; ++i) { + const char* name = sqlite3_column_name(handle, i); + keys.emplace_back(isolate, InternalizedFromUtf8(isolate, name, -1).As()); + names.emplace_back(name); + } + dictionary_template.Reset(); + rows = 0; + if (!CanUseTemplate(names)) names.clear(); + } + + // v8::DictionaryTemplate reads names as Latin-1, so non-ASCII UTF-8 names + // would be garbled, and it crashes on duplicate names and on names that are + // array indices. With kMaxTemplateNames or more names its instances start + // from a dictionary map, which V8 doesn't support there (it DCHECKs), and + // they would be dictionary-mode objects anyway. Rows with such names are + // built with v8::Object::New(). + static bool CanUseTemplate(const std::vector& names) { + if (names.size() >= kMaxTemplateNames) return false; + std::set seen; + for (const std::string& name : names) { + for (const char c : name) { + if (static_cast(c) >= 0x80) return false; + } + if (IsArrayIndex(name) || !seen.insert(name).second) return false; + } + return true; + } + + // Whether `name` is the canonical string of an integer in [0, 2^32 - 2]. + static bool IsArrayIndex(const std::string& name) { + if (name.empty() || name.size() > 10) return false; + if (name.size() > 1 && name[0] == '0') return false; + uint64_t value = 0; + for (const char c : name) { + if (c < '0' || c > '9') return false; + value = value * 10 + static_cast(c - '0'); + } + return value <= 4294967294u; + } + + // V8's JSObject::kMapCacheSize: object literals with this many properties + // get a dictionary map. + static constexpr size_t kMaxTemplateNames = 128; + std::vector> keys; + // The names to create the template from; empty once it is created, or if + // they can't be used for one. + std::vector names; + v8::Global dictionary_template; + int rows = 0; int reprepares; }; diff --git a/src/util/row-builder.cpp b/src/util/row-builder.cpp index 99884c4..9af1556 100644 --- a/src/util/row-builder.cpp +++ b/src/util/row-builder.cpp @@ -3,11 +3,13 @@ class RowBuilder { explicit RowBuilder( v8::Isolate* isolate, + v8::Local ctx, sqlite3_stmt* handle, bool safe_ints, ColumnKeys* column_keys ) : isolate(isolate), + ctx(ctx), handle(handle), column_count(-1), safe_ints(safe_ints), @@ -19,6 +21,19 @@ class RowBuilder { column_keys->Get(isolate, handle, keys); column_count = static_cast(keys.size()); } + if (dictionary_template.IsEmpty()) { + dictionary_template = column_keys->TemplateForRow(isolate); + } + + if (!dictionary_template.IsEmpty()) { + template_values.resize(column_count); + for (int i = 0; i < column_count; ++i) { + template_values[i] = Data::GetValueJS(isolate, handle, i, safe_ints); + } + return dictionary_template->NewInstance(ctx, + v8::MemorySpan>(template_values.data(), template_values.size()) + ); + } v8::LocalVector values(isolate); values.reserve(column_count); @@ -38,9 +53,14 @@ class RowBuilder { private: v8::Isolate* isolate; + v8::Local ctx; sqlite3_stmt* handle; int column_count; const bool safe_ints; ColumnKeys* const column_keys; v8::LocalVector keys; + v8::Local dictionary_template; + // Reused across rows. NewInstance() takes MaybeLocal values, for which + // there is no LocalVector. + std::vector> template_values; }; diff --git a/test/21.statement.get.js b/test/21.statement.get.js index 90258f6..2696d60 100644 --- a/test/21.statement.get.js +++ b/test/21.statement.get.js @@ -108,7 +108,7 @@ describe('Statement#get()', function () { }); it('should use the new column names after the schema changes', function () { const stmt = this.db.prepare('SELECT * FROM entries ORDER BY rowid'); - // Several calls, so later ones reuse the cached column names. + // Several calls, so rows are also built from the cached template. const get = () => { const rows = Array.from({ length: 100 }, () => stmt.get()); for (const row of rows) expect(row).to.deep.equal(rows[0]); @@ -120,4 +120,36 @@ describe('Statement#get()', function () { this.db.prepare('ALTER TABLE entries ADD COLUMN f INTEGER').run(); expect(get()).to.deep.equal({ z: 'foo', b: 1, c: 3.14, d: Buffer.alloc(4).fill(0xdd), e: null, f: null }); }); + it('should build rows like plain objects for unusual column names', function () { + // Several calls, so rows are also built from the cached template. + const row = (sql) => { + const stmt = this.db.prepare(sql); + const results = Array.from({ length: 100 }, () => { + const result = stmt.get(); + expect(Object.getPrototypeOf(result)).to.equal(Object.prototype); + return [Object.getOwnPropertyNames(result), Object.values(result)]; + }); + for (const result of results) expect(result).to.deep.equal(results[0]); + return results[0]; + }; + expect(row('SELECT 1 AS a, 2 AS b, 3 AS a')).to.deep.equal([['a', 'b'], [3, 2]]); + expect(row('SELECT 1 AS "1", 2 AS b, 3 AS "0"')).to.deep.equal([['0', '1', 'b'], [3, 1, 2]]); + expect(row('SELECT 1 AS "__proto__", 2 AS b')).to.deep.equal([['__proto__', 'b'], [1, 2]]); + expect(row('SELECT 1 AS "é", 2 AS "😀", 3 AS "", 4 AS "constructor"')).to.deep.equal([['é', '😀', '', 'constructor'], [1, 2, 3, 4]]); + expect(row('SELECT 1 AS "a""b", 2 AS "c\\d", 3 AS "e\nf"')).to.deep.equal([['a"b', 'c\\d', 'e\nf'], [1, 2, 3]]); + expect(row('SELECT 1 AS "4294967295", 2 AS "-1", 3 AS "01", 4 AS "1.5", 5 AS length')).to.deep.equal([['4294967295', '-1', '01', '1.5', 'length'], [1, 2, 3, 4, 5]]); + }); + it('should build rows with many columns', function () { + for (const width of [127, 128, 300]) { + const names = Array.from({ length: width }, (_, i) => `c${i}`); + const stmt = this.db.prepare(`SELECT ${names.map((name, i) => `${i} AS ${name}`).join(', ')}`); + const expected = Object.fromEntries(names.map((name, i) => [name, i])); + // Several calls, so rows are also built from the cached template. + for (let i = 0; i < 100; ++i) { + const result = stmt.get(); + expect(Object.keys(result)).to.deep.equal(names); + expect(result).to.deep.equal(expected); + } + } + }); }); diff --git a/test/22.statement.all.js b/test/22.statement.all.js index 0a05cdf..bdca0d1 100644 --- a/test/22.statement.all.js +++ b/test/22.statement.all.js @@ -127,7 +127,7 @@ describe('Statement#all()', function () { ).to.throw(RangeError); }); it('should use the new column names after the schema changes', function () { - // Enough rows that the change also shows up past the first ones. + // Many rows, so rows are also built from the cached template. this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 10 FROM entries').run(); this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 20 FROM entries').run(); this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 40 FROM entries').run(); diff --git a/test/23.statement.iterate.js b/test/23.statement.iterate.js index 983b884..5fbb042 100644 --- a/test/23.statement.iterate.js +++ b/test/23.statement.iterate.js @@ -229,7 +229,7 @@ describe('Statement#iterate()', function () { ).to.throw(TypeError); }); it('should use the new column names after the schema changes', function () { - // Enough rows that the change also shows up past the first ones. + // Many rows, so rows are also built from the cached template. this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 10 FROM entries').run(); this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 20 FROM entries').run(); this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 40 FROM entries').run();