-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Add compiler trick to avoid allocation of frozen String literals #1862
Comments
Thanks for adding this. We were just talking about implementing this feature a day or two ago. A couple of notes from that conversation: IR should get a new Operand type of FrozenStringLiteral versus adding a boolean to StringLiteral. @subbuss you were not part of this conversation but you might have thoughts on my notes above. IRBuilder can do this in buildCall but it seems conceptually simpler to have it represented as syntax in AST. |
A new Operand seems good. As for IRBuilder vs. AST, I don't have a preference at this point -- whatever seems simpler. |
This is some seriously non-uniform semantics isn't it? A method call that modifies how the receiver is allocated - crazy. Can't both JRuby and MRI both achieve the same thing with COW? Does it work if freeze is aliased or is it actually a new syntactic rule with priority over normal method call? I'll see what this looks like in Truffle. |
Looks like it is a syntactic rule: 2.1.2 :001 > 5.times{p 'a'.freeze.object_id}
70316340549520
70316340549520
70316340549520
70316340549520
70316340549520
=> 5
2.1.2 :002 > alias :chill :freeze
=> nil
2.1.2 :003 > 5.times{p 'a'.chill.object_id}
70316336878900
70316336878820
70316336878660
70316336878520
70316336878440
=> 5 |
Hmm, I think touching the freeze method breaks the spell, so maybe a guard is needed. |
If it could be done in the AST then I can use the same detection logic from Truffle. If that's not possible don't worry though. |
Chris: good flag reg. syntactic vs. not. I wasn't paying attention. I don't think this can be done syntactically since freeze is just any other method which can be monkey-patched, etc. [ruby-head] [subbu@earth ~] irb |
I can't find specs for this in RubySpec. Perhaps we should do a careful study of what the semantics are and get them into RubySpec before we try to work out how to implement it. There's going to be some very subtle rules here. This is the MRI implementation ruby/ruby@07ac587. I don't really know MRI internals but does it look like when you call For JRuby we also need to be careful about a race condition here. If two threads run the call site for the first time you may still get two different string objects. Does Rubinius do it yet does anyone know? In Truffle I guess I will add similar logic to my dispatch nodes, which will replace the string literal node with an object literal of the frozen string plus an assumption to deoptimize if the method is redefined. Shouldn't have any runtime overhead at all. I saw @tenderlove's presentation on some of these optimisation at Brighton Ruby Conf, and my general opinion was that this is all stuff better left for the compiler to do transparently. For example with these strings I think it might be better to logically allocate them, and then rely on a compiler phase to later remove them. I know that's not really possible in MRI as it is at the moment, but it is in all other implementations. |
@chrisseaton ouch. I did not know they added a redefinition guard with it. This does not reflect my understanding of how this optimization would work. Originally, people wanted this as a syntactical optimization but they also wanted it to still parse in older Ruby impls. I never remember anyone talking about a redefinition guard. I would even be willing to accept it as syntactical and wait to see if anyone ever notices. I would also be willing to raise this as an issue to MRI since I feel this defeats the original intent. |
Thinking about this a little more even with a simple guard which would at worst be a simple object/primitive value comparison would still be way faster than reboxing a RubyString instance with a COW backend (which is what all Ruby Strings are now). My bigger problem with this is that we cannot make use of this immutable string knowledge because of the guard. |
This is still open but it appears that it was implemented at some point. I haven't gone through the changelog to identify which release, but:
Possibly this could be closed? |
Good catch, thanks @nehresma! I am going to mark this as fixed in 9.1, because before that it was only partially implemented. |
Hi!
I just saw this presentation which mentions the Ruby 2.1 trick to re-use immutable String objects for frozen String literals:
https://www.youtube.com/watch?feature=player_detailpage&v=d2QdITRRMHg#t=3570
It is heavily used in Rails and JRuby should take advantage of it.
Example code:
With Ruby 2.1:
With JRuby 1.7.13:
With JRuby master:
The text was updated successfully, but these errors were encountered: