From b4ec0e979c9f09531c0e93ccb2ee565dc3dcdc09 Mon Sep 17 00:00:00 2001
From: Charles Oliver Nutter <headius@headius.com>
Date: Tue, 14 May 2024 16:37:59 +0900
Subject: [PATCH] More string interpolation optimizations

* Don't re-copy the buffer when creating initial buffer from first
  static string.
* Log how the dstring has been bound by indy
* Additional cases in benchmark
* Benchmark reuses proc to omit block creation overhead.
* Stringable returns same frozen string to avoid new string cost.
---
 bench/bench_block_given.rb                    |  5 +++--
 bench/bench_string_interpolation.rb           | 21 ++++++++++++++++++-
 .../targets/indy/BuildDynamicStringSite.java  | 18 ++++++++++++++--
 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/bench/bench_block_given.rb b/bench/bench_block_given.rb
index 205e1ad6317..2f5bd7dffb4 100644
--- a/bench/bench_block_given.rb
+++ b/bench/bench_block_given.rb
@@ -9,10 +9,11 @@ def foo2
 end
 
 Benchmark.ips do |bm|
+  pr = proc { }
   bm.report("block") do |i|
     while i > 0
       i-=1
-      foo { }; foo { }; foo { }; foo { }; foo { }; foo { }; foo { }; foo { }; foo { }; foo { }
+      foo &pr; foo &pr; foo &pr; foo &pr; foo &pr; foo &pr; foo &pr; foo &pr; foo &pr; foo &pr
     end
   end
 
@@ -26,7 +27,7 @@ def foo2
   bm.report("defined block") do |i|
     while i > 0
       i-=1
-      foo2 { }; foo2 { }; foo2 { }; foo2 { }; foo2 { }; foo2 { }; foo2 { }; foo2 { }; foo2 { }; foo2 { }
+      foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr; foo2 &pr
     end
   end
 
diff --git a/bench/bench_string_interpolation.rb b/bench/bench_string_interpolation.rb
index 31a6c13c422..329fee72f8c 100644
--- a/bench/bench_string_interpolation.rb
+++ b/bench/bench_string_interpolation.rb
@@ -2,7 +2,7 @@
 
 class Stringable
   def to_s
-    "quux"
+    "quux".freeze
   end
 end
 
@@ -96,6 +96,15 @@ def to_s
     a
   }
 
+  bm.report('"foo#{n}bar#{n}baz#{n}" fixnum') {|n|
+    a = nil
+    while n > 0
+      n-=1
+      a = "foo#{n}bar#{n}baz"
+    end
+    a
+  }
+
   bm.report('"#{x}#{x}#{x}#{x}#{x}" to_s') {|n|
     a = nil
     x = Stringable.new
@@ -105,4 +114,14 @@ def to_s
     end
     a
   }
+
+  bm.report('"#{x}#{x}#{x}#{x}#{x}#{x}#{x}#{x}#{x}#{x}" to_s') {|n|
+    a = nil
+    x = Stringable.new
+    while n > 0
+      n-=1
+      a = "#{x}#{x}#{x}#{x}#{x}#{x}#{x}#{x}#{x}#{x}"
+    end
+    a
+  }
 }
diff --git a/core/src/main/java/org/jruby/ir/targets/indy/BuildDynamicStringSite.java b/core/src/main/java/org/jruby/ir/targets/indy/BuildDynamicStringSite.java
index 1f6bbba79f2..7c438341ea2 100644
--- a/core/src/main/java/org/jruby/ir/targets/indy/BuildDynamicStringSite.java
+++ b/core/src/main/java/org/jruby/ir/targets/indy/BuildDynamicStringSite.java
@@ -9,6 +9,9 @@
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.util.ByteList;
 import org.jruby.util.StringSupport;
+import org.jruby.util.cli.Options;
+import org.jruby.util.log.Logger;
+import org.jruby.util.log.LoggerFactory;
 import org.objectweb.asm.Handle;
 import org.objectweb.asm.Opcodes;
 
@@ -24,6 +27,8 @@
 import static org.jruby.util.CodegenUtils.sig;
 
 public class BuildDynamicStringSite extends MutableCallSite {
+    private static final Logger LOG = LoggerFactory.getLogger(BuildDynamicStringSite.class);
+
     public static final Handle BUILD_DSTRING_BOOTSTRAP = new Handle(
             Opcodes.H_INVOKESTATIC,
             p(BuildDynamicStringSite.class),
@@ -94,13 +99,22 @@ public BuildDynamicStringSite(MethodType type, Object[] stringArgs) {
 
         if (specialize) {
             // bind directly to specialized builds
+            if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
+                LOG.info("dstring(" + Long.toBinaryString(descriptor) +")" + "\tbound directly");
+            }
             binder = binder.append(arrayOf(Encoding.class, int.class), encoding, initialSize);
             setTarget(binder.invokeStaticQuiet(BuildDynamicStringSite.class, "buildString"));
         } else if (dynamicArgs <= MAX_DYNAMIC_ARGS_FOR_SPECIALIZE2) {
             // second level specialization, using call site to hold strings, no argument[] box, and appending in a loop
+            if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
+                LOG.info("dstring(" + Long.toBinaryString(descriptor) +")" + "\tbound to unrolled loop");
+            }
             binder = binder.prepend(this).append(arrayOf(Encoding.class, int.class), encoding, initialSize);
             setTarget(binder.invokeVirtualQuiet("buildString2"));
         } else {
+            if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
+                LOG.info("dstring(" + Long.toBinaryString(descriptor) +")" + "\tbound to loop");
+            }
             binder = binder.prepend(this).collect(2, IRubyObject[].class);
             setTarget(binder.invokeVirtualQuiet("buildString"));
         }
@@ -462,9 +476,9 @@ private static RubyString createBufferFromStaticString(ThreadContext context, in
         byte[] bufferArray = Arrays.copyOfRange(firstStringByteList.unsafeBytes(), firstStringByteList.begin(), initialSize);
 
         // use element realSize for starting buffer realSize
-        ByteList bufferByteList = new ByteList(bufferArray, 0, firstStringByteList.realSize());
+        ByteList bufferByteList = new ByteList(bufferArray, 0, firstStringByteList.realSize(), firstStringByteList.getEncoding(), false);
 
-        buffer = RubyString.newStringNoCopy(context.runtime, bufferByteList, firstStringByteList.getEncoding(), firstStringCR);
+        buffer = RubyString.newString(context.runtime, bufferByteList, firstStringCR);
         return buffer;
     }