Pre-dedup literal hash string keys but not all string keys.

Fixes #3472. There's further work here to move the notion of a frozen string into the parser, so the IR build can simply run as normal without special cases for different types of keys. That will come with 2.3 work to enable all literal strings to be pre-frozen.
jruby · Nov 20, 2015 · f3f0091 · f3f0091
1 parent da88890
commit f3f0091
Showing 3 changed files with 12 additions and 3 deletions.
diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java
@@ -1015,7 +1015,7 @@ protected void op_asetForString(Ruby runtime, RubyString key, IRubyObject value)
             entry.value = value;
         } else {
             checkIterating();
-            if (!key.isFrozen()) key = runtime.freezeAndDedupString(key);
+            if (!key.isFrozen()) key = (RubyString)key.dupFrozen();
             internalPut(key, value, false);
         }
     }
@@ -1026,7 +1026,7 @@ protected void op_asetSmallForString(Ruby runtime, RubyString key, IRubyObject v
             entry.value = value;
         } else {
             checkIterating();
-            if (!key.isFrozen()) key = runtime.freezeAndDedupString(key);
+            if (!key.isFrozen()) key = (RubyString)key.dupFrozen();
             internalPutSmall(key, value, false);
         }
     }

diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -2531,7 +2531,14 @@ public Operand buildHash(HashNode hashNode) {
                 addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, splat}));
                 continue;
             } else {
-                keyOperand = buildWithOrder(key, hasAssignments);
+                // TODO: This isn't super pretty. If AST were aware of literal hash string keys being "special"
+                // it could have an appropriate AST node for frozen string and this code would just go away.
+                if (key instanceof StrNode) {
+                    StrNode strKey = (StrNode)key;
+                    keyOperand = new FrozenString(strKey.getValue(), strKey.getCodeRange());
+                } else {
+                    keyOperand = buildWithOrder(key, hasAssignments);
+                }
             }
 
             args.add(new KeyValuePair<>(keyOperand, buildWithOrder(pair.getValue(), hasAssignments)));

diff --git a/core/src/main/java/org/jruby/ir/operands/FrozenString.java b/core/src/main/java/org/jruby/ir/operands/FrozenString.java
@@ -15,6 +15,8 @@
  * This is not an immutable literal because I can gsub!,
  * for example, and modify the contents of the string.
  * This is not like a Java string.
+ *
+ * TODO: This really should be ImmutableLiteral so it can constant propagate, etc.
  */
 public class FrozenString extends StringLiteral {
     /**