-
-
Notifications
You must be signed in to change notification settings - Fork 48
feat: Implement Nitro's getExternalMemorySize
#261
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
Changes from all commits
be227e3
cbad82e
b4dc19f
b6d4497
9bd2bf8
00816fb
e3062f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,60 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace margelo::nitro::rnnitrosqlite { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Compute the approximate external memory size of a single result row. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * This includes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Column name string capacities, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Heap usage for the actual SQLiteValue contents. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t getRowExternalMemorySize(const SQLiteQueryResultRow& row) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t bucketMemory = row.bucket_count() * sizeof(void*); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constexpr size_t nodePadding = 24; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t nodesMemory = row.size() * (sizeof(std::pair<std::string, SQLiteValue>) * nodePadding); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bucketMemory + nodesMemory; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Compute the approximate external memory size of the full result set. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * We add: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - The vector's backing storage, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - All rows (column names + values). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t getResultsExternalMemorySize(const SQLiteQueryResults& results) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t size = sizeof(SQLiteQueryResults); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const auto resultCapacity = results.capacity(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size += resultCapacity * sizeof(SQLiteQueryResultRow); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const auto& row : results) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size += getRowExternalMemorySize(row); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+34
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Variants are not containers - they are unions with paddings and indexers. Even if it holds a small value (like a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh, good to know and makes sense in terms of memory allocation. Thanks! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return size; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Compute the approximate external memory size of the table metadata. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * We include: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Column name string capacities (map keys), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Metadata contents, especially the `name` string on each metadata entry. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t getMetadataExternalMemorySize(const SQLiteQueryTableMetadata& metadata) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t size = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const auto& [columnName, columnMeta] : metadata) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size += columnName.capacity(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size += columnMeta.name.capacity(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return size; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } // namespace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::optional<double> HybridNitroSQLiteQueryResult::getInsertId() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _insertId; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -24,4 +78,16 @@ std::optional<SQLiteQueryTableMetadata> HybridNitroSQLiteQueryResult::getMetadat | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _metadata; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t HybridNitroSQLiteQueryResult::getExternalMemorySize() noexcept { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t size = sizeof(*this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size += getResultsExternalMemorySize(_results); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (_metadata) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size += getMetadataExternalMemorySize(*_metadata); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return size; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } // namespace margelo::nitro::rnnitrosqlite | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Hi! I've been testing out this library locally, and I'm getting console errors with
Error: NitroSQLite.execute(...): External memory is too high.I was able to track this down to this line. I believe the
* nodePaddinghere should be+ nodePaddinginstead?(This was the easiest way for me to report this issue. Apologies for just directly commenting on an already-merged PR.)
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.
Hey @sleeper-luke, thank you for the catch, this is a valid bug. Addressed in #287