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

Add compiler trick to avoid allocation of frozen String literals #1862

Closed
donv opened this issue Jul 26, 2014 · 12 comments
Closed

Add compiler trick to avoid allocation of frozen String literals #1862

donv opened this issue Jul 26, 2014 · 12 comments

Comments

@donv
Copy link
Member

donv commented Jul 26, 2014

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:

2.1.2 :002 > 5.times{p 'a'.object_id}
70119468854480
70119468854360
70119468854280
70119468854200
70119468854120
 => 5 
2.1.2 :003 > 5.times{p 'a'.freeze.object_id}
70119476885680
70119476885680
70119476885680
70119476885680
70119476885680
 => 5 

With JRuby 1.7.13:

jruby-1.7.13 :001 > 5.times{p 'a'.freeze.object_id}
2042
2044
2046
2048
2050
 => 5 

With JRuby master:

jruby-head :001 > 5.times{p 'a'.freeze.object_id}
2048
2050
2052
2054
2056
 => 5 
@donv donv added this to the JRuby 9000 milestone Jul 26, 2014
@enebo
Copy link
Member

enebo commented Jul 26, 2014

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.
I would like to make this something which can be detected during parsing instead of having IRBuilder detect this. This can probably be a FrozenStrNode? I believe support.new_call can replace any StrNode as receiver with a FrozenStrNode.

@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.

@subbuss
Copy link
Contributor

subbuss commented Jul 26, 2014

A new Operand seems good. As for IRBuilder vs. AST, I don't have a preference at this point -- whatever seems simpler.

@chrisseaton
Copy link
Contributor

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.

@donv
Copy link
Member Author

donv commented Jul 26, 2014

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 

@donv
Copy link
Member Author

donv commented Jul 26, 2014

Hmm, I think touching the freeze method breaks the spell, so maybe a guard is needed.

@chrisseaton
Copy link
Contributor

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.

@subbuss
Copy link
Contributor

subbuss commented Jul 27, 2014

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
ruby-head :001 > 2.times { p 'a'.freeze.object_id }
18567220
18567220
=> 2
ruby-head :002 > class String; def freeze; p 'boo!'; end; end
=> :freeze
ruby-head :003 > 2.times { p 'a'.freeze.object_id }
"boo!"
18860680
"boo!"
18860260
=> 2

@chrisseaton
Copy link
Contributor

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 freeze on a string literal for the first time, it checks if freeze is redefined, and if not it replaces the instruction with one that returns the same string in the future. Then there is a guard that checks there is still no redefinition, but it looks like the instructions stays there either way. So you are losing the allocation but gaining a new guard. For MRI that tradeoff is probably fine, but if the JVM was optimising your strings away anyway we need to check the guard is also optimised away otherwise it's going to worse performance, not better.

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.

@enebo
Copy link
Member

enebo commented Jul 27, 2014

@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.

@enebo
Copy link
Member

enebo commented Jul 27, 2014

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.

@enebo enebo modified the milestone: JRuby 9.0.0.0 Jul 14, 2015
@nehresma
Copy link

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:

jruby-9.1.1.0 :001 > 5.times{p 'a'.freeze.object_id}
2048
2048
2048
2048
2048
 => 5 
jruby-9.1.1.0 :002 > 5.times{p 'a'.object_id}
2050
2052
2054
2056
2058

Possibly this could be closed?

@headius
Copy link
Member

headius commented May 31, 2016

Good catch, thanks @nehresma! I am going to mark this as fixed in 9.1, because before that it was only partially implemented.

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

No branches or pull requests

6 participants