Skip to content
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

Make Result#fields return interned strings in Ruby 3+ #1181

Merged
merged 1 commit into from
May 19, 2021

Conversation

casperisfine
Copy link
Contributor

Context

These strings are very likely to be used for hash keys. And when using a mutable string as a hash key, ruby will duplicate it and freeze the copy:

>> k = "foo" + "bar"
=> "foobar"
>> k.object_id
=> 260
>> h = {k => 1}
=> {"foobar"=>1}
>> h.keys.first.object_id
=> 280

This leads to performance patches like this one: rails/rails@a46dcb7

Additionally, on recent rubies, when using a frozen string as a key, Ruby will intern the string to save on memory: msgpack/msgpack-ruby@e8dedef

And finally, since Ruby 3.0, rb_enc_interned_str allow to directly retrieve an existing interned string without allocating any object.

This patch

Based on the three statements above, I believe that it would be preferable to directly intern the field string if possible, and if not to simply freeze them.

Of course this could cause some minor compatibility issues, but they quite unlikely and trivial to fix.

@sodabrew
Copy link
Collaborator

This generally makes sense to me. Interned strings are not garbage collected until Ruby 2.2 and higher, however this gem supports Ruby 2.0 and higher. I would be concerned if the same string is interned multiple times, does it reuse the pre-existing intern or create another one? I'd want to be sure it's not going to create a memory leak for applications.

That said, if this does result in a memory leak but only for latest mysql2 gem releases on Ruby 2.0 and 2.1, then it would be sensible to set Ruby 2.2 as the new minimum version.

@casperisfine
Copy link
Contributor Author

Interned strings are not garbage collected until Ruby 2.2 and higher

The API used here is only available in 3.0 and higher. For older rubies there basically no change.

does it reuse the pre-existing intern or create another one?

Always re-use yes.

@sodabrew
Copy link
Collaborator

The subtle change is freezing the strings on older Rubies. It's an important relnote.

@sodabrew sodabrew merged commit ca883e1 into brianmario:master May 19, 2021
@casperisfine
Copy link
Contributor Author

The subtle change is freezing the strings on older Rubies

Indeed, I forgot to point it out, this is for the interface to be consistent with newer rubies (because interned string implies frozen).

Thanks a lot for the merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants