Two allocation reductions in result materialisation - #1460
Open
nuclearspike wants to merge 1 commit into
Open
Conversation
Two independent changes, both about memory the current code reserves or reshuffles without needing to. ### 1. Do not reserve the rows array when rows are not cached `rb_mysql_result_each` sizes `wrapper->rows` to the full result with `rb_ary_new2(numberOfRows)` unconditionally. Under `cache_rows: false` nothing is ever stored in that array -- rows are yielded and dropped -- so the reservation is dead weight that scales with the result size. Streaming a 2,000,000-row result with `cache_rows: false`, measuring `ObjectSpace.memsize_of(rows)` on the result's internal array: | | reserved | |---|---| | master | 16,000,040 bytes | | patch | 40 bytes | `cache_rows: false` exists precisely so that large results do not have to be held in memory, which is the case where reserving room for every row is least welcome. `rb_ary_new2(n)` sets capacity, not length, so nothing observable changes in the result's contents: the array is empty either way. The reservation is visible to memory introspection, which is exactly what the table above uses. ### 2. Pre-size the row hash to the column count Row hashes are built with `rb_hash_new()` and then filled one key per column, so a row wider than the default capacity rehashes while it is being built -- once per row, for the whole result. `rb_hash_new_capa(numberOfFields)` sizes it up front. 5,000 rows x 42 columns, `as: :hash`, materialisation only. Min of 9 runs per sample, 11 samples per arm, interleaved: | | median | range | |---|---|---| | master | 24.44 ms | 23.96 - 26.27 | | patch | 22.58 ms | 21.77 - 24.50 | -7.6%, though the distributions do overlap at the edges, so treat that as the size of the effect rather than a precise figure. `rb_hash_new_capa` is Ruby 3.2+, so it is behind `have_func` and older builds keep `rb_hash_new()`. #### The case where this costs memory instead of saving it `numberOfFields` is the right capacity only when the column names are distinct. mysql2 keys rows by field name, so duplicate names overwrite each other and the finished hash can be far smaller than the column count -- while the pre-sized allocation still reserves for every column. Measured on a 200-column result whose columns all share one alias, so one key survives: | | `memsize_of(row_hash)` | |---|---| | master | 160 bytes | | patch | 7,248 bytes | Per row, and retained under `cache_rows: true`. In the ordinary case of distinct names the patch is slightly better than master (7,248 vs 7,328 bytes for 200 unique columns), because it avoids the growth overshoot -- but the pathological shape is a genuine regression and it seemed wrong to report only the favourable measurement. `SELECT *` across joined tables that share column names is the realistic way to hit it, though hitting it hard needs many collisions rather than a handful. The alternative that keeps both properties is to size later rows from the first row's actual key count rather than from the field count, at the cost of carrying that count on the result. ### Where this was tested Ruby 3.3.10, MySQL 9.6.0 (Homebrew, libmysqlclient.24), macOS 26.5 arm64, Apple clang 21. The memory figures above are `ObjectSpace.memsize_of` on that build; the exact byte counts are Ruby-build specific, the ratios are the point. The rspec suite is green apart from three failures already present on the base commit: `client_spec.rb:101`, `:729`, and `:740` -- the reconnect group. Not tested: MariaDB, Windows, 32-bit platforms, or other Ruby versions. Both changes are Ruby-side allocation only and do not depend on the client library.
Collaborator
|
Nice! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent changes, both about memory the current code reserves or
reshuffles without needing to.
1. Do not reserve the rows array when rows are not cached
rb_mysql_result_eachsizeswrapper->rowsto the full result withrb_ary_new2(numberOfRows)unconditionally. Undercache_rows: falsenothingis ever stored in that array -- rows are yielded and dropped -- so the
reservation is dead weight that scales with the result size.
Streaming a 2,000,000-row result with
cache_rows: false, measuringObjectSpace.memsize_of(rows)on the result's internal array:cache_rows: falseexists precisely so that large results do not have to beheld in memory, which is the case where reserving room for every row is least
welcome.
rb_ary_new2(n)sets capacity, not length, so nothing observablechanges in the result's contents: the array is empty either way. The reservation
is visible to memory introspection, which is exactly what the table above uses.
2. Pre-size the row hash to the column count
Row hashes are built with
rb_hash_new()and then filled one key per column,so a row wider than the default capacity rehashes while it is being built --
once per row, for the whole result.
rb_hash_new_capa(numberOfFields)sizes itup front.
5,000 rows x 42 columns,
as: :hash, materialisation only. Min of 9 runs persample, 11 samples per arm, interleaved:
-7.6%, though the distributions do overlap at the edges, so treat that as the
size of the effect rather than a precise figure.
rb_hash_new_capais Ruby 3.2+, so it is behindhave_funcand older buildskeep
rb_hash_new().The case where this costs memory instead of saving it
numberOfFieldsis the right capacity only when the column names are distinct.mysql2 keys rows by field name, so duplicate names overwrite each other and the
finished hash can be far smaller than the column count -- while the pre-sized
allocation still reserves for every column. Measured on a 200-column result
whose columns all share one alias, so one key survives:
memsize_of(row_hash)Per row, and retained under
cache_rows: true. In the ordinary case of distinctnames the patch is slightly better than master (7,248 vs 7,328 bytes for 200
unique columns), because it avoids the growth overshoot -- but the pathological
shape is a genuine regression and it seemed wrong to report only the favourable
measurement.
SELECT *across joined tables that share column names is therealistic way to hit it, though hitting it hard needs many collisions rather
than a handful.
The alternative that keeps both properties is to size later rows from the first
row's actual key count rather than from the field count, at the cost of carrying
that count on the result.
Where this was tested
Ruby 3.3.10, MySQL 9.6.0 (Homebrew, libmysqlclient.24), macOS 26.5 arm64,
Apple clang 21. The memory figures above are
ObjectSpace.memsize_ofon thatbuild; the exact byte counts are Ruby-build specific, the ratios are the point.
The rspec suite is green apart from three failures already
present on the base commit:
client_spec.rb:101,:729, and:740-- thereconnect group.
Not tested: MariaDB, Windows, 32-bit platforms, or other Ruby versions. Both
changes are Ruby-side allocation only and do not depend on the client library.
Relationship to #1422: it also reduces allocation in result fetching and would conflict textually with this — its new
CAST_NONE/CAST_FASTbranches each allocate a row hash withrb_hash_new(), which is what change 2 here pre-sizes on the existing path. The two look complementary rather than competing (its fast paths would want the same pre-sizing), and I'm happy to rebase this on top of it, or drop change 2 entirely if you'd rather take it there. It doesn't touchwrapper->rows, so change 1 is independent either way.If the duplicate-column memory case matters, the fix that keeps both properties is to size later rows from the first row's actual key count rather than the field count — a few lines, but it adds per-result state, so I didn't want to assume. Happy to add it if wanted.
🤖 Generated with Claude Code