Fix to support Ruby 3.0 Ractor #22
Conversation
| SCHEME_LIST_MUTEX.synchronize do | ||
| const_set(:SCHEMES, ObjectSpace. | ||
| each_object(Class). | ||
| select { |klass| klass < URI::Generic }. |
There was a problem hiding this comment.
This seems very expensive, iterate the whole heap when previously it was just registered directly.
Could the Class#inherited hook be used instead at least?
Or maybe use Class#descendants (checking if available with respond_to?) from https://bugs.ruby-lang.org/issues/14394?
In #15 I think the way was clearly to use instance variables for this, and that has been accepted in https://bugs.ruby-lang.org/issues/17592. And the main Ractor could simply write to that instance variable. That has not been implemented yet though (cc @ko1).
Another strategy might be to haves SCHEMES = {}, and then once the first URI is created, Ractor.make_shareable(SCHEMES) unless SCHEMES.frozen?. That would prevent adding new schemes after the first URI is created, but maybe it's good enough, and it feels like it fits well with the Ractor model.
Adding support for Ractor by making everything slower/more hacky is IMHO absolutely not acceptable.
| end | ||
|
|
||
| # Re-calculate scheme list | ||
| def self.refresh_scheme_list |
There was a problem hiding this comment.
This is yet another hack, now the API exposes the caching to the end user, that's leaking implementation details.
It's ugly, there is no other word.
|
IMHO this should be reverted, or improved in a way that:
|
|
Yet another possibility, since we are going to break |
|
I worked on a clean solution in #26 |
* This reverts commit 1faa4fdc161d7aeebdb5de0c407b923beaecf898. * It has too many problems, see ruby/uri#22 for discussion. ruby/uri@b959da2dc9
This PR is targeting to solve the same problem as #15 and some ideas are similar, but in contrast this one does not have the problem with multiple constant removal on the start up
https://bugs.ruby-lang.org/issues/17592