From 306a16cd0b0fbfdb14dac85a3ac6675ed0d506d4 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 24 Aug 2023 12:27:37 -0500 Subject: [PATCH] Move frozen string debug fields into subclass This avoids adding anything to the ivar table and short-circuits the frozen check. We may want to consider doing this for non-debug frozen literal strings as well. Note: This removes the debug check from "DStr" logic in BuildCompoundStringInstr because I could find no case where CRuby attaches debug info to a dynamically-constructed frozen string, and removing it did not appear to affect any tests. --- core/src/main/java/org/jruby/RubyString.java | 40 ++++++++++++------- .../src/main/java/org/jruby/ir/IRBuilder.java | 9 ++--- .../BuildCompoundStringInstr.java | 11 ++--- .../jruby/ir/runtime/IRRuntimeHelpers.java | 24 +---------- 4 files changed, 34 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java index aa02b109ace..fc9456e7c06 100644 --- a/core/src/main/java/org/jruby/RubyString.java +++ b/core/src/main/java/org/jruby/RubyString.java @@ -120,8 +120,6 @@ */ @JRubyClass(name="String", include={"Enumerable", "Comparable"}) public class RubyString extends RubyObject implements CharSequence, EncodingCapable, MarshalEncoding, CodeRangeable { - public static final String DEBUG_INFO_FIELD = "@debug_created_info"; - static final ASCIIEncoding ASCII = ASCIIEncoding.INSTANCE; static final UTF8Encoding UTF8 = UTF8Encoding.INSTANCE; @@ -956,20 +954,8 @@ private void modifyCheck(byte[] b, int len, Encoding enc) { if (value.getUnsafeBytes() != b || value.getRealSize() != len || value.getEncoding() != enc) throw getRuntime().newRuntimeError("string modified"); } - private void frozenCheck() { + protected void frozenCheck() { if (isFrozen()) { - if (getRuntime().getInstanceConfig().isDebuggingFrozenStringLiteral()) { - IRubyObject obj = getInstanceVariable(DEBUG_INFO_FIELD); - - if (obj != null && obj instanceof RubyArray) { - RubyArray info = (RubyArray) obj; - if (info.getLength() == 2) { - throw getRuntime().newRaiseException(getRuntime().getFrozenError(), - "can't modify frozen String, created at " + info.eltInternal(0) + ":" + info.eltInternal(1)); - } - } - } - throw getRuntime().newFrozenError("String", this); } } @@ -1052,6 +1038,30 @@ public RubyString newFrozen() { return str; } + public static RubyString newDebugFrozenString(Ruby runtime, RubyClass rubyClass, ByteList value, int cr, String file, int line) { + return new DebugFrozenString(runtime, runtime.getString(), value, cr, file, line); + } + + static class DebugFrozenString extends RubyString { + private final String file; + private final int line; + + protected DebugFrozenString(Ruby runtime, RubyClass rubyClass, ByteList value, int cr, String file, int line) { + super(runtime, rubyClass, value, cr); + + this.file = file; + this.line = line; + } + + @Override + protected void frozenCheck() { + Ruby runtime = getRuntime(); + + throw runtime.newRaiseException(runtime.getFrozenError(), + "can't modify frozen String, created at " + file + ":" + line); + } + } + /** rb_str_resize */ public final void resize(final int size) { diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java index bd21410d5a3..e2715926b3b 100644 --- a/core/src/main/java/org/jruby/ir/IRBuilder.java +++ b/core/src/main/java/org/jruby/ir/IRBuilder.java @@ -1598,7 +1598,7 @@ private void maybeGenerateIsNotEmptyErrorString(Variable errorString, Operand re label("empty", (empty) -> cond(empty, result, tru(), ()-> addInstr(new BuildCompoundStringInstr(errorString, new Operand[] {value, new FrozenString(" is not empty")}, - UTF8Encoding.INSTANCE, 13, true, false, getFileName(), lastProcessedLineNum)))); + UTF8Encoding.INSTANCE, 13, true, getFileName(), lastProcessedLineNum)))); } public interface RunIt { @@ -3418,8 +3418,7 @@ public Operand buildDStr(Variable result, DStrNode node) { if (result == null) result = createTemporaryVariable(); - boolean debuggingFrozenStringLiteral = manager.getInstanceConfig().isDebuggingFrozenStringLiteral(); - addInstr(new BuildCompoundStringInstr(result, pieces, node.getEncoding(), estimatedSize, node.isFrozen(), debuggingFrozenStringLiteral, getFileName(), node.getLine())); + addInstr(new BuildCompoundStringInstr(result, pieces, node.getEncoding(), estimatedSize, node.isFrozen(), getFileName(), node.getLine())); return result; } @@ -3436,7 +3435,7 @@ public Operand buildDSymbol(Variable result, DSymbolNode node) { if (result == null) result = createTemporaryVariable(); boolean debuggingFrozenStringLiteral = manager.getInstanceConfig().isDebuggingFrozenStringLiteral(); - addInstr(new BuildCompoundStringInstr(result, pieces, node.getEncoding(), estimatedSize, false, debuggingFrozenStringLiteral, getFileName(), node.getLine())); + addInstr(new BuildCompoundStringInstr(result, pieces, node.getEncoding(), estimatedSize, false, getFileName(), node.getLine())); return copy(new DynamicSymbol(result)); } @@ -3458,7 +3457,7 @@ public Operand buildDXStr(Variable result, DXStrNode node) { if (result == null) result = createTemporaryVariable(); boolean debuggingFrozenStringLiteral = manager.getInstanceConfig().isDebuggingFrozenStringLiteral(); - addInstr(new BuildCompoundStringInstr(stringResult, pieces, node.getEncoding(), estimatedSize, false, debuggingFrozenStringLiteral, getFileName(), node.getLine())); + addInstr(new BuildCompoundStringInstr(stringResult, pieces, node.getEncoding(), estimatedSize, false, getFileName(), node.getLine())); return fcall(result, Self.SELF, "`", stringResult); } diff --git a/core/src/main/java/org/jruby/ir/instructions/BuildCompoundStringInstr.java b/core/src/main/java/org/jruby/ir/instructions/BuildCompoundStringInstr.java index bfa38d7b9fc..4eb224462cc 100644 --- a/core/src/main/java/org/jruby/ir/instructions/BuildCompoundStringInstr.java +++ b/core/src/main/java/org/jruby/ir/instructions/BuildCompoundStringInstr.java @@ -23,17 +23,15 @@ public class BuildCompoundStringInstr extends NOperandResultBaseInstr { final private Encoding encoding; final private boolean frozen; - final private boolean debug; final private String file; final private int line; private final int estimatedSize; - public BuildCompoundStringInstr(Variable result, Operand[] pieces, Encoding encoding, int estimatedSize, boolean frozen, boolean debug, String file, int line) { + public BuildCompoundStringInstr(Variable result, Operand[] pieces, Encoding encoding, int estimatedSize, boolean frozen, String file, int line) { super(Operation.BUILD_COMPOUND_STRING, result, pieces); this.encoding = encoding; this.frozen = frozen; - this.debug = debug; this.file = file; this.line = line; this.estimatedSize = estimatedSize; @@ -53,7 +51,7 @@ public int getInitialSize() { @Override public Instr clone(CloneInfo ii) { - return new BuildCompoundStringInstr(ii.getRenamedVariable(result), cloneOperands(ii), encoding, estimatedSize, frozen, debug, file, line); + return new BuildCompoundStringInstr(ii.getRenamedVariable(result), cloneOperands(ii), encoding, estimatedSize, frozen, file, line); } @Override @@ -69,7 +67,7 @@ public void encode(IRWriterEncoder e) { public static BuildCompoundStringInstr decode(IRReaderDecoder d) { boolean debuggingFrozenStringLiteral = d.getCurrentScope().getManager().getInstanceConfig().isDebuggingFrozenStringLiteral(); - return new BuildCompoundStringInstr(d.decodeVariable(), d.decodeOperandArray(), d.decodeEncoding(), d.decodeInt(), d.decodeBoolean(), debuggingFrozenStringLiteral, d.decodeString(), d.decodeInt()); + return new BuildCompoundStringInstr(d.decodeVariable(), d.decodeOperandArray(), d.decodeEncoding(), d.decodeInt(), d.decodeBoolean(), d.decodeString(), d.decodeInt()); } @Override @@ -90,9 +88,6 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco } if (frozen) { - if (debug) { - return IRRuntimeHelpers.freezeLiteralString(str, context, file, line); - } return IRRuntimeHelpers.freezeLiteralString(str); } return str; diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java index 8073b8aa9a4..09a9dbbcf0c 100644 --- a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java +++ b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java @@ -2370,18 +2370,11 @@ public static Binding newFrameScopeBinding(ThreadContext context, IRubyObject se public static RubyString newFrozenString(ThreadContext context, ByteList bytelist, int coderange, String file, int line) { Ruby runtime = context.runtime; - RubyString string = RubyString.newString(runtime, bytelist, coderange); - if (runtime.getInstanceConfig().isDebuggingFrozenStringLiteral()) { - // stuff location info into the string and then freeze it - RubyArray info = (RubyArray) runtime.newArray(runtime.newString(file).freeze(context), runtime.newFixnum(line + 1)).freeze(context); - string.setInstanceVariable(RubyString.DEBUG_INFO_FIELD, info); - string.setFrozen(true); - } else { - string = runtime.freezeAndDedupString(string); + return RubyString.newDebugFrozenString(runtime, runtime.getString(), bytelist, coderange, file, line); } - return string; + return runtime.freezeAndDedupString(RubyString.newString(runtime, bytelist, coderange)); } @JIT @Interp @@ -2391,19 +2384,6 @@ public static RubyString freezeLiteralString(RubyString string) { return string; } - @JIT @Interp - public static RubyString freezeLiteralString(RubyString string, ThreadContext context, String file, int line) { - Ruby runtime = context.runtime; - - if (runtime.getInstanceConfig().isDebuggingFrozenStringLiteral()) { - // stuff location info into the string and then freeze it - RubyArray info = (RubyArray) runtime.newArray(runtime.newString(file).freeze(context), runtime.newFixnum(line + 1)).freeze(context); - string.setInstanceVariable(RubyString.DEBUG_INFO_FIELD, info); - } - - return freezeLiteralString(string); - } - @JIT public static IRubyObject callOptimizedAref(ThreadContext context, IRubyObject caller, IRubyObject target, RubyString keyStr, CallSite site) { if (target instanceof RubyHash &&