ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: "matz (Yukihiro Matsumoto) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "matz (Yukihiro Matsumoto)" <noreply@ruby-lang.org>
Subject: [ruby-core:119054] [Ruby master Feature#20684] Add optimized instructions for frozen literal Hash and Array
Date: Thu, 05 Sep 2024 05:50:29 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-109631.20240905055028.8880@ruby-lang.org> (raw)
In-Reply-To: <redmine.issue-20684.20240819092024.8880@ruby-lang.org>

Issue #20684 has been updated by matz (Yukihiro Matsumoto).


Basically agreed. But the existence (and side effect) of this optimization should be implementation defined, especially regarding `object_id`.

Matz.


----------------------------------------
Feature #20684: Add optimized instructions for frozen literal Hash and Array
https://bugs.ruby-lang.org/issues/20684#change-109631

* Author: etienne (Étienne Barrié)
* Status: Open
----------------------------------------
# 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

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

PR: https://github.com/ruby/ruby/pull/11406



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

  reply	other threads:[~2024-09-05  5:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19  9:20 [ruby-core:118891] " etienne via ruby-core
2024-09-05  5:50 ` matz (Yukihiro Matsumoto) via ruby-core [this message]
2024-09-05  6:08 ` [ruby-core:119055] " byroot (Jean Boussier) via ruby-core

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-all from there: mbox

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

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

  git send-email \
    --in-reply-to=redmine.journal-109631.20240905055028.8880@ruby-lang.org \
    --to=ruby-core@ml.ruby-lang.org \
    --cc=noreply@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 NNTP newsgroup(s).