Skip to content

Commit

Permalink
Move frozen string debug fields into subclass
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@headius
headius committed Aug 24, 2023
1 parent d032f1d commit 306a16c
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 50 deletions.
40 changes: 25 additions & 15 deletions core/src/main/java/org/jruby/RubyString.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 4 additions & 5 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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));
}
Expand All @@ -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);
}
Expand Down
11 changes: 3 additions & 8 deletions core/src/main/java/org/jruby/ir/instructions/BuildCompoundStringInstr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
Expand Down
24 changes: 2 additions & 22 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 &&
Expand Down

0 comments on commit 306a16c

Please sign in to comment.