ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: tenderlove@ruby-lang.org
To: ruby-core@ruby-lang.org
Subject: [ruby-core:91092] [Ruby trunk Feature#15393] Add compilation flags to freeze Array and Hash literals
Date: Tue, 15 Jan 2019 00:15:59 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-76323.20190115001558.ec41cf32843c1eb9@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-15393.20181208004237@ruby-lang.org

Issue #15393 has been updated by tenderlovemaking (Aaron Patterson).


Eregon (Benoit Daloze) wrote:
> tenderlovemaking (Aaron Patterson) wrote:
> > I thought about doing this with ".freeze", introducing a special instruction the same way we do for the `"string".freeze` optimization, but dealing with deoptization in the case someone monkey patches the method seems like a pain.
> 
> I think it might semantically make some sense to ignore monkey-patching of `.freeze` (and `.deep_freeze`) for calls on literals, which would then avoid needing deoptimization for this (although deoptimization would be a flag check + the current logic as fallback, it doesn't seem so bad).
> It's somewhat similar to String literals not calling String#initialize for instance (same for Array, Hash).
> And it's probably a bad idea to override #freeze in the hope it would be used on frozen literals anyway, as of course this would only work if the monkey-patch is reliably loaded before everything else.
> 
> Another issue is it's quite ugly to opt out of frozen array/hash literals, if a mutable copy is wanted:
> 
> ```ruby
> # frozen_hash_and_array_literal: true
> my_mutable_data = { 'a' => ['b', { 'c' => 'd' }.dup].dup }.dup
> ```
> 
> That's why I think `deep_freeze` would better express the intent in some cases, and be finer-grained:
> 
> ```ruby
> MY_CONSTANT = { 'a' => ['b', { 'c' => 'd' }] }.deep_freeze
> my_mutable_data = { 'a' => ['b', { 'c' => 'd' }] }
> 
> { :defaults => 'thing' }.deep_freeze.merge(some_hash)
> ```

Yes, I totally agree.  I think `deep_freeze` is going to be necessary for adoption of Guilds, and the code I'm proposing in this patch would be an optimization of the `deep_freeze` call on a literal.

> and this would work regardless of what is the value to freeze (but only avoid allocations if it's all literals, otherwise a constant must be used).
> 
> Furthermore, frozen literals don't allow composition or extraction in different constants:
> ```ruby
> # frozen_hash_and_array_literal: true
> MY_KEY = 'a'
> MY_CONSTANT = { MY_KEY => ['b', { 'c' => 'd' }] } # Not frozen, and breaks referential transparency
> 
> MY_CONSTANT = { MY_KEY => ['b', { 'c' => 'd' }] }.deep_freeze # Works
> ```
> 
> OTOH, "string".freeze has shown the magic comment is much nicer in many cases than adding "string".freeze in many places (at the price of making a mutable String not so nice, but those seem rarer).
> It would be interesting to get an idea of what's a typical ratio of immutable/mutable Array and Hash literals, and what converting a codebase to use frozen Array/Hash literals would look like.

Right.  I'm not proposing adding the magic comment at all, just adding a parameter to the ISeq constructor that will allow you to "deep freeze" literals in the code passed to ISeq#new.


----------------------------------------
Feature #15393: Add compilation flags to freeze Array and Hash literals
https://bugs.ruby-lang.org/issues/15393#change-76323

* Author: tenderlovemaking (Aaron Patterson)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Hi,

I would like to add VM compilation options to freeze array and hash literals.  For example:

~~~ ruby
frozen = RubyVM::InstructionSequence.compile(<<-eocode, __FILE__, nil, 0, frozen_string_literal: true, frozen_hash_and_array_literal: true)
  { 'a' => ['b', { 'c' => 'd' }] }
eocode
puts frozen.disasm
~~~

Output is:

~~~
$ ./ruby thing.rb
== disasm: #<ISeq:<compiled>@thing.rb:0 (0,0)-(0,34)> (catch: FALSE)
0000 putobject                    {"a"=>["b", {"c"=>"d"}]}
0002 leave
~~~

Anything nested in the hash that can't be "frozen" will cause it to not be frozen.

For example:

~~~ ruby
not_frozen = RubyVM::InstructionSequence.compile(<<-eocode, __FILE__, nil, 0, frozen_string_literal: true, frozen_hash_and_array_literal: true)
  { 'a' => some_method }
eocode
puts not_frozen.disasm
~~~

Output:

~~~
$ ./ruby thing.rb
== disasm: #<ISeq:<compiled>@thing.rb:0 (0,0)-(0,24)> (catch: FALSE)
0000 putobject                    "a"
0002 putself
0003 opt_send_without_block       <callinfo!mid:some_method, argc:0, FCALL|VCALL|ARGS_SIMPLE>, <callcache>
0006 newhash                      2
0008 leave
~~~

Eventually I would like to freeze array and hash literals in source code itself, but I think this is a good first step.

The reason I want this feature is I think we can reduce some object allocations, and once Guilds are implemented, easily create immutable data.

I've attached a patch that implements the above.

(Also I think maybe "frozen_literals" would be a better name, but I don't want to imply that numbers or booleans are frozen too)

Thanks!



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

      parent reply	other threads:[~2019-01-15  0:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <redmine.issue-15393.20181208004237@ruby-lang.org>
2018-12-08  0:42 ` [ruby-core:90375] [Ruby trunk Feature#15393] Add compilation flags to freeze Array and Hash literals tenderlove
2018-12-08 13:58 ` [ruby-core:90379] " shevegen
2018-12-08 17:58 ` [ruby-core:90380] " eregontp
2018-12-10  0:55 ` [ruby-core:90387] " shyouhei
2018-12-10 13:07   ` [ruby-core:90404] " Benoit Daloze
2018-12-10 20:10 ` [ruby-core:90408] " tenderlove
2018-12-12 21:35 ` [ruby-core:90457] " shannonskipper
2018-12-13  8:47 ` [ruby-core:90500] " janfri26
2018-12-14 19:51 ` [ruby-core:90530] " nardonykolyszyn
2018-12-15 13:27 ` [ruby-core:90549] " eregontp
2018-12-18 17:08 ` [ruby-core:90605] " 223-322-223-322
2018-12-18 17:15 ` [ruby-core:90606] " 223-322-223-322
2019-01-15  0:15 ` tenderlove [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-76323.20190115001558.ec41cf32843c1eb9@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).