Ensure all ImmutableRubyString have a Rope from the RopeCache

Author: eregon

src/main/java/org/truffleruby/RubyLanguage.java

@@ -18,6 +18,7 @@
 import com.oracle.truffle.api.instrumentation.AllocationReporter;
 import com.oracle.truffle.api.object.Shape;
 import org.graalvm.options.OptionDescriptors;
+import org.jcodings.Encoding;
 import org.truffleruby.builtins.PrimitiveManager;
 import org.truffleruby.core.RubyHandle;
 import org.truffleruby.core.array.RubyArray;
@@ -53,7 +54,7 @@
 import org.truffleruby.core.range.RubyObjectRange;
 import org.truffleruby.core.regexp.RubyMatchData;
 import org.truffleruby.core.regexp.RubyRegexp;
-import org.truffleruby.core.rope.LeafRope;
+import org.truffleruby.core.rope.CodeRange;
 import org.truffleruby.core.rope.Rope;
 import org.truffleruby.core.rope.RopeCache;
 import org.truffleruby.core.string.CoreStrings;
@@ -153,9 +154,9 @@ public final class RubyLanguage extends TruffleLanguage<RubyContext> {
     public final CoreStrings coreStrings;
     public final CoreSymbols coreSymbols;
     public final PrimitiveManager primitiveManager;
-    public final FrozenStringLiterals frozenStringLiterals;
     public final RopeCache ropeCache;
     public final SymbolTable symbolTable;
+    public final FrozenStringLiterals frozenStringLiterals;
     @CompilationFinal public LanguageOptions options;
 
     @CompilationFinal private AllocationReporter allocationReporter;
@@ -212,9 +213,9 @@ public RubyLanguage() {
         coreStrings = new CoreStrings(this);
         coreSymbols = new CoreSymbols();
         primitiveManager = new PrimitiveManager();
-        frozenStringLiterals = new FrozenStringLiterals();
         ropeCache = new RopeCache(coreSymbols);
         symbolTable = new SymbolTable(ropeCache, coreSymbols);
+        frozenStringLiterals = new FrozenStringLiterals(ropeCache);
     }
 
     @TruffleBoundary
@@ -435,7 +436,11 @@ public AllocationReporter getAllocationReporter() {
         return allocationReporter;
     }
 
-    public ImmutableRubyString getFrozenStringLiteral(LeafRope rope) {
+    public ImmutableRubyString getFrozenStringLiteral(byte[] bytes, Encoding encoding, CodeRange codeRange) {
+        return frozenStringLiterals.getFrozenStringLiteral(bytes, encoding, codeRange);
+    }
+
+    public ImmutableRubyString getFrozenStringLiteral(Rope rope) {
         return frozenStringLiterals.getFrozenStringLiteral(rope);
     }
 

src/main/java/org/truffleruby/core/CoreLibrary.java

@@ -44,7 +44,6 @@
 import org.truffleruby.core.numeric.BigIntegerOps;
 import org.truffleruby.core.numeric.RubyBignum;
 import org.truffleruby.core.rope.CodeRange;
-import org.truffleruby.core.rope.LeafRope;
 import org.truffleruby.core.string.RubyString;
 import org.truffleruby.core.string.StringOperations;
 import org.truffleruby.debug.GlobalVariablesObject;
@@ -737,10 +736,8 @@ private void setConstant(RubyModule module, String name, Object value) {
     }
 
     private ImmutableRubyString frozenUSASCIIString(String string) {
-        // NOTE(norswap, Nov. 2nd 2020): Okay for language access to be slow, currently only used during initialization.
-        final LeafRope rope = language.ropeCache.getRope(
+        return language.getFrozenStringLiteral(
                 StringOperations.encodeRope(string, USASCIIEncoding.INSTANCE, CodeRange.CR_7BIT));
-        return language.getFrozenStringLiteral(rope);
     }
 
     private RubyClass defineClass(String name) {

src/main/java/org/truffleruby/core/encoding/EncodingManager.java

@@ -32,7 +32,6 @@
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.klass.RubyClass;
 import org.truffleruby.core.rope.CodeRange;
-import org.truffleruby.core.rope.LeafRope;
 import org.truffleruby.core.rope.Rope;
 import org.truffleruby.core.rope.RopeOperations;
 import org.truffleruby.core.string.EncodingUtils;
@@ -182,11 +181,7 @@ private RubyEncoding newRubyEncoding(Encoding encoding, byte[] name, int p, int
                 "; name.length = " + name.length + ")";
 
         final Rope rope = RopeOperations.create(name, USASCIIEncoding.INSTANCE, CodeRange.CR_7BIT);
-        final LeafRope cachedRope = language.ropeCache.getRope(
-                rope.getBytes(),
-                rope.getEncoding(),
-                rope.getCodeRange());
-        final ImmutableRubyString string = language.getFrozenStringLiteral(cachedRope);
+        final ImmutableRubyString string = language.getFrozenStringLiteral(rope);
 
         final RubyEncoding instance = new RubyEncoding(
                 context.getCoreLibrary().encodingClass,

src/main/java/org/truffleruby/core/rope/RopeCache.java

@@ -62,10 +62,6 @@ public LeafRope getRope(Rope string) {
         return getRope(string.getBytes(), string.getEncoding(), string.getCodeRange());
     }
 
-    public LeafRope getRope(Rope string, CodeRange codeRange) {
-        return getRope(string.getBytes(), string.getEncoding(), codeRange);
-    }
-
     @TruffleBoundary
     public LeafRope getRope(byte[] bytes, Encoding encoding, CodeRange codeRange) {
         assert encoding != null;

src/main/java/org/truffleruby/core/string/FrozenStringLiterals.java

@@ -9,20 +9,35 @@
  */
 package org.truffleruby.core.string;
 
+import org.jcodings.Encoding;
 import org.truffleruby.collections.WeakValueCache;
+import org.truffleruby.core.rope.CodeRange;
 import org.truffleruby.core.rope.LeafRope;
 import org.truffleruby.core.rope.Rope;
+import org.truffleruby.core.rope.RopeCache;
 
 public class FrozenStringLiterals {
 
-    private final WeakValueCache<Rope, ImmutableRubyString> values = new WeakValueCache<>();
+    private final RopeCache ropeCache;
+    private final WeakValueCache<LeafRope, ImmutableRubyString> values = new WeakValueCache<>();
 
-    public ImmutableRubyString getFrozenStringLiteral(LeafRope rope) {
-        final ImmutableRubyString string = values.get(rope);
+    public FrozenStringLiterals(RopeCache ropeCache) {
+        this.ropeCache = ropeCache;
+    }
+
+    public ImmutableRubyString getFrozenStringLiteral(Rope rope) {
+        return getFrozenStringLiteral(rope.getBytes(), rope.getEncoding(), rope.getCodeRange());
+    }
+
+    public ImmutableRubyString getFrozenStringLiteral(byte[] bytes, Encoding encoding, CodeRange codeRange) {
+        // Ensure all ImmutableRubyString have a Rope from the RopeCache
+        final LeafRope cachedRope = ropeCache.getRope(bytes, encoding, codeRange);
+
+        final ImmutableRubyString string = values.get(cachedRope);
         if (string != null) {
             return string;
         } else {
-            return values.addInCacheIfAbsent(rope, new ImmutableRubyString(rope));
+            return values.addInCacheIfAbsent(cachedRope, new ImmutableRubyString(cachedRope));
         }
     }
 

src/main/java/org/truffleruby/core/string/ImmutableRubyString.java

@@ -28,7 +28,7 @@
 import org.truffleruby.language.library.RubyStringLibrary;
 
 /** All ImmutableRubyString are interned and must be created through
- * {@link FrozenStringLiterals#getFrozenStringLiteral(LeafRope)}. */
+ * {@link FrozenStringLiterals#getFrozenStringLiteral(Rope)}. */
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(RubyStringLibrary.class)
 public class ImmutableRubyString extends ImmutableRubyObject implements TruffleObject {

src/main/java/org/truffleruby/core/string/StringNodes.java

@@ -5370,7 +5370,7 @@ public abstract static class InternNode extends PrimitiveArrayArgumentsNode {
         @Specialization
         protected ImmutableRubyString internString(RubyString string,
                 @Cached RopeNodes.FlattenNode flattenNode) {
-            final LeafRope flattened = flattenNode.executeFlatten(string.rope);
+            final Rope flattened = flattenNode.executeFlatten(string.rope);
             return getLanguage().getFrozenStringLiteral(flattened);
         }
 

src/main/java/org/truffleruby/parser/BodyTranslator.java

@@ -59,7 +59,6 @@
 import org.truffleruby.core.regexp.RegexpOptions;
 import org.truffleruby.core.regexp.RubyRegexp;
 import org.truffleruby.core.regexp.TruffleRegexpNodes;
-import org.truffleruby.core.rope.CodeRange;
 import org.truffleruby.core.rope.LeafRope;
 import org.truffleruby.core.rope.Rope;
 import org.truffleruby.core.rope.RopeConstants;
@@ -511,10 +510,9 @@ public RubyNode visitCallNode(CallParseNode node) {
         if (receiver instanceof StrParseNode && methodName.equals("freeze")) {
             final StrParseNode strNode = (StrParseNode) receiver;
             final Rope nodeRope = strNode.getValue();
-            final CodeRange codeRange = strNode.getCodeRange();
 
-            final LeafRope rope = language.ropeCache.getRope(nodeRope, codeRange);
-            final ImmutableRubyString frozenString = language.getFrozenStringLiteral(rope);
+            final ImmutableRubyString frozenString = language
+                    .getFrozenStringLiteral(nodeRope.getBytes(), nodeRope.getEncoding(), strNode.getCodeRange());
 
             return addNewlineIfNeeded(node, withSourceSection(
                     sourceSection,
@@ -2936,17 +2934,18 @@ public RubyNode visitSplatNode(SplatParseNode node) {
 
     @Override
     public RubyNode visitStrNode(StrParseNode node) {
-        final LeafRope rope = language.ropeCache.getRope(node.getValue(), node.getCodeRange());
+        final Rope nodeRope = node.getValue();
         final RubyNode ret;
 
         if (node.isFrozen()) {
-            final ImmutableRubyString frozenString = language.getFrozenStringLiteral(rope);
+            final ImmutableRubyString frozenString = language
+                    .getFrozenStringLiteral(nodeRope.getBytes(), nodeRope.getEncoding(), node.getCodeRange());
 
-            ret = new DefinedWrapperNode(
-                    language.coreStrings.EXPRESSION,
-                    new ObjectLiteralNode(frozenString));
+            ret = new DefinedWrapperNode(language.coreStrings.EXPRESSION, new ObjectLiteralNode(frozenString));
         } else {
-            ret = new StringLiteralNode(rope);
+            final LeafRope cachedRope = language.ropeCache
+                    .getRope(nodeRope.getBytes(), nodeRope.getEncoding(), node.getCodeRange());
+            ret = new StringLiteralNode(cachedRope);
         }
         ret.unsafeSetSourceSection(node.getPosition());
         return addNewlineIfNeeded(node, ret);