Project

General

Profile

Actions

Feature #20350

closed

Return chilled string from Symbol#to_s

Added by Dan0042 (Daniel DeLorme) 9 months ago. Updated about 1 month ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:117240]

Description

During Ruby 2.7 development there was an attempt to return a frozen string from Symbol#to_s (#16150#note-22)

This had to be rolled back due to incompatibility, but now we have chilled strings (#20205)

Symbol#to_s can safely return a chilled string, giving developers time to fix warnings before switching to a frozen string.


Files

Screenshot 2024-11-06 at 14.00.47.png (118 KB) Screenshot 2024-11-06 at 14.00.47.png jhawthorn (John Hawthorn), 11/06/2024 10:02 PM

Updated by byroot (Jean Boussier) 9 months ago

To provide some data-point. Our monolith runs with https://github.com/Shopify/symbol-fstring, which makes the return value of Symbol#to_s frozen without any problem. I think we had to fix a single gem to make it work a few years back, that's it.

That said, while we run a lot of code (700+ transitive gems), that's not necessarily fully representative of the code out there.

Also one major difference with chilled string literals is that you can't just set RUBYOPT="--disable-frozen-string-literal" to continue running older code. I guess you could monkey patch String#to_s though, and provide that as a gem to make it easy to provide backward compatibility.

All this to say that as a Ruby user, I'd like this change to happen, because Symbol#to_s is a major source of allocation, and Ruby performance is very GC dependent.

But as a Ruby committer I think we need to be careful about the rate of deprecation and backward compatibility breaks. On one hand it would sound logical to ship such change at the same time as the change for frozen string literals, but on the other hand it makes the jump bigger and may alienate some users. So it's a fine line to walk.

So I'm positive, but with some reservations.

Updated by Eregon (Benoit Daloze) 9 months ago

+1, I was thinking the same yesterday, that we could revisit Symbol#to_s and have a proper deprecation before changing it due to chilled strings landing.

Updated by ko1 (Koichi Sasada) 8 months ago

Symbol#name returns frozen string.
It is not enough?

Updated by byroot (Jean Boussier) 8 months ago

Symbol#name returns frozen string. It is not enough?

It doesn't work well with duck typing. It's common to have a method that call to_s on the argument, and the argument can be a Symbol or another type:

def some_method(arg)
  arg = arg.to_s
  # do things
end

We can debate the merits of such design of course, but from my experience it's extremely common. Years after Symbol#name introduction, Symbol#to_s remains visible on production profiles, and running https://github.com/Shopify/symbol-fstring/ still provide a small, but noticeable improvement.

Updated by matz (Yukihiro Matsumoto) 8 months ago

I am in favor of experimenting. I am in favor of moving to immutable strings in the future if there are no significant incompatibility issues.

Matz.

Updated by jhawthorn (John Hawthorn) about 1 month ago

As one data point in favour of this we swapped Symbol#to_s for Symbol#name (Symbol.alias_method :to_s, :name) to GitHub.com last week and it reduced allocations by about 5% on our web requests. So there should be pretty significant savings to users if we can make this default.

Updated by Eregon (Benoit Daloze) about 1 month ago

Matz already approved this change above, so I think the only missing thing is a PR to implement it.

It might be late to add this in 3.4 though, but it could be merged early in 3.5 dev.

Updated by byroot (Jean Boussier) about 1 month ago

We wrote the implementation with @etienne (Étienne Barrié) this morning: https://github.com/ruby/ruby/pull/12065

No idea if we wish to include this for 3.4 or not though. People are already complaining quite a bit about various incompatibilities (Hash#inspect, Backtrace::Location#to_s, etc), so we might want to wait January.

Actions #10

Updated by Anonymous about 1 month ago

  • Status changed from Open to Closed

Applied in changeset git|6deeec5d459ecff5ec4628523b14ac7379fd942e.


Mark strings returned by Symbol#to_s as chilled (#12065)

  • Use FL_USER0 for ELTS_SHARED

This makes space in RString for two bits for chilled strings.

  • Mark strings returned by Symbol#to_s as chilled

[Feature #20350]

STR_CHILLED now spans on two user flags. If one bit is set it
marks a chilled string literal, if it's the other it marks a
Symbol#to_s chilled string.

Since it's not possible, and doesn't make much sense to include
debug info when --debug-frozen-string-literal is set, we can't
include allocation source, but we can safely include the symbol
name in the warning message, making it much easier to find the source
of the issue.

Co-Authored-By: Étienne Barrié


Co-authored-by: Étienne Barrié
Co-authored-by: Jean Boussier

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like1Like0Like0Like0