Skip to content

Add optimized instructions for frozen literal Hash and Array#11406

Merged
byroot merged 2 commits intoruby:masterfrom
Shopify:opt_ary_freeze
Sep 5, 2024
Merged

Add optimized instructions for frozen literal Hash and Array#11406
byroot merged 2 commits intoruby:masterfrom
Shopify:opt_ary_freeze

Conversation

@etiennebarrie
Copy link
Copy Markdown
Contributor

[Feature #20684]

Context

Methods that take empty arrays or empty hashes as default values allocate a new object each time the method is called without the argument. Often they don't mutate the parameter. To prevent an allocation, in performance critical sections, a constant is defined that holds a frozen hash or array, and the constant is defined as the default value for the parameter.

Here are some examples:
Rails: https://github.com/rails/rails/blob/607d61e884237c223c24c6f47efa0b561dd8b637/activerecord/lib/active_record/relation/query_methods.rb#L159-L160
Roda: https://github.com/jeremyevans/roda/blob/102926a02dcabc9a31674e3cf98f049139c31492/lib/roda/plugins.rb#L9-L10
dry-rb: https://github.com/dry-rb/dry-container/blob/1ee41bb109455d06bf22ebcbd94b050cc4773733/lib/dry/container/mixin.rb#L68C5-L68C15
and many other gems: https://gist.github.com/casperisfine/47f22243d4ad203855256ef5bfae7979

Additionally when defining a frozen literal constant, we're currently inefficient because we store the literal in the bytecode, we dup it just to freeze it again. It doesn't amount to much but would be nice to avoid.

Proposal

This PRs introduces 2 new optimized instructions opt_ary_freeze and opt_hash_freeze that behave like opt_str_freeze for their respective types. If the freeze method hasn't been redefined, they simply push the frozen literal value on the stack. Like for opt_str_freeze, these instructions are added by the peephole optimizer when applicable.

In the specific case of empty array and empty hash, we use a pre-allocated global empty frozen object to avoid retaining a distinct empty object each time.

This will allow code like this: https://github.com/ruby/ruby/blob/566f2eb501d94d4047a9aad4af0d74c6a96f34a9/lib/rubygems/resolver/api_set/gem_parser.rb to be shortened and simplified like this:

diff --git i/lib/rubygems/resolver/api_set/gem_parser.rb w/lib/rubygems/resolver/api_set/gem_parser.rb
index 643b857107..34146fd426 100644
--- i/lib/rubygems/resolver/api_set/gem_parser.rb
+++ w/lib/rubygems/resolver/api_set/gem_parser.rb
@@ -1,15 +1,12 @@
 # frozen_string_literal: true
 
 class Gem::Resolver::APISet::GemParser
-  EMPTY_ARRAY = [].freeze
-  private_constant :EMPTY_ARRAY
-
   def parse(line)
     version_and_platform, rest = line.split(" ", 2)
     version, platform = version_and_platform.split("-", 2)
     dependencies, requirements = rest.split("|", 2).map! {|s| s.split(",") } if rest
-    dependencies = dependencies ? dependencies.map! {|d| parse_dependency(d) } : EMPTY_ARRAY
-    requirements = requirements ? requirements.map! {|d| parse_dependency(d) } : EMPTY_ARRAY
+    dependencies = dependencies ? dependencies.map! {|d| parse_dependency(d) } : [].freeze
+    requirements = requirements ? requirements.map! {|d| parse_dependency(d) } : [].freeze
     [version, platform, dependencies, requirements]
   end

Overall it's a minor optimization but also a very simple patch and makes code nicer.

@matzbot matzbot requested a review from a team August 19, 2024 09:21
@ko1 ko1 added the Playground Experimental: Post link to try Ruby with PR changes label Sep 5, 2024
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 5, 2024

Try on Playground: https://ruby.github.io/play-ruby?run=10718146951
This is an automated comment by pr-playground.yml workflow.

@byroot byroot enabled auto-merge (rebase) September 5, 2024 09:57
etiennebarrie and others added 2 commits September 5, 2024 11:58
If an Array which is empty or only using literals is frozen, we detect
this as a peephole optimization and change the instructions to be
`opt_ary_freeze`.

[Feature #20684]

Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
If a Hash which is empty or only using literals is frozen, we detect
this as a peephole optimization and change the instructions to be
`opt_hash_freeze`.

[Feature #20684]

Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
auto-merge was automatically disabled September 5, 2024 09:58

Head branch was pushed to by a user without write access

@byroot byroot merged commit bf98797 into ruby:master Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Playground Experimental: Post link to try Ruby with PR changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants