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

Freeze strings and constants in pragmas #620

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

tenderlove
Copy link
Member

I'm looking in earnest at Ractor support in Rails. We need some of these constants that SQLite3 uses to be frozen so that when they're accessed via Ractors we don't get an error.

This commit just adds frozen string literals, and freezes some constants

I'm looking in earnest at Ractor support in Rails. We need some of these
constants that SQLite3 uses to be frozen so that when they're accessed
via Ractors we don't get an error.

This commit just adds frozen string literals, and freezes some constants
@flavorjones flavorjones merged commit d7e98cd into sparklemotion:main Mar 6, 2025
136 checks passed
@flavorjones
Copy link
Member

@tenderlove LMK if you need this in a release.

@flavorjones
Copy link
Member

Side note: the freezing idiom here is kinda gross. Is there no better way to do this, like Ractor.make_shareable or # shareable_constant_value: literal?

I read through Feature #17145: Ractor-aware Object#deep_freeze and Feature #17274: Ractor.make_shareable(obj) and I also skimmed ruby/doc/ractor.md I'm still not sure I understand how this is supposed to be done.

@tenderlove
Copy link
Member Author

@tenderlove LMK if you need this in a release.

I do not. I'm working off edge here, so it's fine.

Side note: the freezing idiom here is kinda gross. Is there no better way to do this, like Ractor.make_shareable or # shareable_constant_value: literal?

I didn't want to use the Ractor constant because then the code isn't backwards compatible (or we'd have to do work to make it backwards compatible). I think the comment directive might work, but IIRC there's a problem with it (that I can't remember off the top of my head). I just recall an issue while fixing a bug in Prism (not an issue with Prism, but a problem with how the directive works in practice).

I agree we're calling freeze a bunch. Maybe we should try to figure out why the data structure is nested so much? It looks like some of these constants are truly tuples, so maybe we could use a hash. I'm not sure why the other ones are nested arrays. I'll look in to it.

@tenderlove tenderlove deleted the freeze branch March 7, 2025 17:21
@flavorjones
Copy link
Member

I didn't want to use the Ractor constant because then the code isn't backwards compatible

@tenderlove This gem only supports Ruby >= 3.1, and as far as I can tell Ractor.make_shareable was introduced into Ruby 3.0. So I'm not worried about backwards compat if you want to use it instead of freeze.

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.

2 participants