Skip to content

Avoid use of id2ref for weak mapping #35

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

Merged
merged 1 commit into from
Apr 28, 2025
Merged

Avoid use of id2ref for weak mapping #35

merged 1 commit into from
Apr 28, 2025

Conversation

kou
Copy link
Member

@kou kou commented Apr 25, 2025

[Bugs #15711]

https://bugs.ruby-lang.org/issues/15711

The implementation of ObjectSpace._id2ref on JRuby (and TruffleRuby) is very expensive, and on JRuby we normally do not have it enabled because it impacts performance of the entire runtime.

This uses ObjectSpace::WeakMap instead of ObjectSpace._id2ref by default and deprecates DRb::WeakIdConv.

Authored-by: Masatoshi SEKI [email protected]

@kou
Copy link
Member Author

kou commented Apr 25, 2025

Should we add jobs for JRuby and TruffleRuby?

@headius
Copy link

headius commented Apr 25, 2025

@kou I would be very happy to see a JRuby job. I'll see if there's anything needed to make it green today.

@headius
Copy link

headius commented Apr 25, 2025

There's a few failures so far. I'm not sure if they are real incompatibilities on JRuby or if they are just slight behavioral differences.

@headius
Copy link

headius commented Apr 25, 2025

I'll open a separate issue with JRuby test suite failures.

[Bugs #15711]

https://bugs.ruby-lang.org/issues/15711

The implementation of `ObjectSpace._id2ref` on JRuby (and TruffleRuby)
is very expensive, and on JRuby we normally do not have it enabled
because it impacts performance of the entire runtime.

This uses `ObjectSpace::WeakMap` instead of `ObjectSpace._id2ref` by
default and deprecates `DRb::WeakIdConv`.

Authored-by: Masatoshi SEKI <[email protected]>
@kou
Copy link
Member Author

kou commented Apr 28, 2025

OK. Let's work on enabling JRuby in CI as a separated task.

@kou
Copy link
Member Author

kou commented Apr 28, 2025

The separated issue: #36

@kou kou merged commit b360d74 into ruby:master Apr 28, 2025
23 checks passed
@kou
Copy link
Member Author

kou commented Apr 28, 2025

Oh, I should have used "Co-authored-by" not "Authored-by"...

@kou kou deleted the drb-id-conv branch April 28, 2025 08:09
@yahonda
Copy link

yahonda commented May 21, 2025

Would you consider releasing a new version of dRuby that includes this change?

ObjectSpace._id2ref has been deprecated via ruby/ruby#13157, and as a result, Rails CI against the Ruby master branch is flooded with warnings:
https://buildkite.com/rails/rails-nightly/builds/2253#0196ef7f-b098-46dc-928f-46c83bc5758e/1244-1254

@kou
Copy link
Member Author

kou commented May 21, 2025

Done.

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