Skip to content

Commit a695fb1

Browse files
eregoneregon
committed
[GR-32001] Always-inline Kernel#respond_to? and Kernel#respond_to_missing?
* Use core symbols instead of core strings for internal calls to respond_to?, that way if we need to call respond_to_missing? there is no extra conversion, and conversion to java.lang.String is fast.
1 parent 48ce7d0 commit a695fb1

File tree

9 files changed

+81
-108
lines changed

9 files changed

+81
-108
lines changed

‎spec/truffle/always_inlined_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@
2020
}.should raise_error(TypeError) { |e| e.backtrace_locations[0].label.should == 'public_send' }
2121
end
2222

23+
it "for #respond_to?" do
24+
-> {
25+
respond_to?()
26+
}.should raise_error(ArgumentError) { |e| e.backtrace_locations[0].label.should == 'respond_to?' }
27+
-> {
28+
respond_to?(Object.new)
29+
}.should raise_error(TypeError) { |e| e.backtrace_locations[0].label.should == 'respond_to?' }
30+
end
31+
2332
it "for #block_given?" do
2433
-> {
2534
block_given?(:wrong)
@@ -191,6 +200,19 @@
191200
end
192201

193202
guard -> { RUBY_ENGINE != "ruby" } do
203+
it "for #respond_to?" do
204+
obj = Object.new
205+
def obj.respond_to_missing?(name, priv)
206+
name == :foo ? raise("foo") : super
207+
end
208+
-> {
209+
obj.respond_to?(:foo)
210+
}.should raise_error(RuntimeError, "foo") { |e|
211+
e.backtrace_locations[0].label.should == 'respond_to_missing?'
212+
e.backtrace_locations[1].label.should.start_with?('block (5 levels)')
213+
}
214+
end
215+
194216
it "for Method#call" do
195217
def method_to_call
196218
raise "foo"

‎src/main/java/org/truffleruby/core/array/ArrayNodes.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.graalvm.collections.EconomicSet;
2222
import org.graalvm.collections.Equivalence;
2323
import org.truffleruby.Layouts;
24-
import org.truffleruby.RubyLanguage;
2524
import org.truffleruby.builtins.CoreMethod;
2625
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
2726
import org.truffleruby.builtins.CoreMethodNode;
@@ -1263,7 +1262,7 @@ protected RubyArray initializeFromArray(
12631262
guards = { "!isImplicitLong(object)", "wasProvided(object)", "!isRubyArray(object)" })
12641263
protected RubyArray initialize(RubyArray array, Object object, NotProvided unusedValue, Nil block) {
12651264
RubyArray copy = null;
1266-
if (respondToToAry(getLanguage(), object)) {
1265+
if (respondToToAry(object)) {
12671266
Object toAryResult = callToAry(object);
12681267
if (toAryResult instanceof RubyArray) {
12691268
copy = (RubyArray) toAryResult;
@@ -1278,13 +1277,12 @@ protected RubyArray initialize(RubyArray array, Object object, NotProvided unuse
12781277
}
12791278
}
12801279

1281-
public boolean respondToToAry(RubyLanguage language, Object object) {
1280+
public boolean respondToToAry(Object object) {
12821281
if (respondToToAryNode == null) {
12831282
CompilerDirectives.transferToInterpreterAndInvalidate();
1284-
respondToToAryNode = insert(KernelNodesFactory.RespondToNodeFactory.create(null, null, null));
1283+
respondToToAryNode = insert(KernelNodesFactory.RespondToNodeFactory.create());
12851284
}
1286-
return respondToToAryNode
1287-
.executeDoesRespondTo(null, object, language.coreStrings.TO_ARY.createInstance(getContext()), true);
1285+
return respondToToAryNode.executeDoesRespondTo(object, coreSymbols().TO_ARY, true);
12881286
}
12891287

12901288
protected Object callToAry(Object object) {

‎src/main/java/org/truffleruby/core/inlined/AlwaysInlinedMethodNode.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
import org.truffleruby.language.control.RaiseException;
2020

2121
/** A core method that should always be executed inline, without going through a CallTarget. That enables accessing the
22-
* caller frame efficiently and reliably.
22+
* caller frame efficiently and reliably. It also means there is a copy/"split" of that core method for every call site.
2323
* <p>
2424
* If called from a foreign language, then the caller frame will be null. The node should check using
25-
* {@link #needCallerFrame(Frame, RootCallTarget)} that the caller frame is not null before using it (if it uses it), in
26-
* order to provide a useful exception in that case.
25+
* {@link #needCallerFrame(Frame, RootCallTarget)} that the caller frame is not null before using it (if it needs it),
26+
* in order to provide a useful exception in that case.
2727
* <p>
2828
* Such a method will not appear in backtraces. However, Ruby exceptions emitted from this node will be resent through a
29-
* CallTarget to get the proper backtrace.
29+
* CallTarget to get the proper backtrace. This should be tested in spec/truffle/always_inlined_spec.rb.
3030
* <p>
3131
* Such a core method should not emit significantly more Graal nodes than a non-inlined call, as Truffle cannot decide
3232
* to not inline it, and that could lead to too big methods to compile. */

‎src/main/java/org/truffleruby/core/kernel/KernelNodes.java

Lines changed: 41 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@
103103
import org.truffleruby.language.RubySourceNode;
104104
import org.truffleruby.language.WarnNode;
105105
import org.truffleruby.language.WarningNode;
106-
import org.truffleruby.language.arguments.ReadCallerFrameNode;
107106
import org.truffleruby.language.arguments.RubyArguments;
108107
import org.truffleruby.language.backtrace.Backtrace;
109108
import org.truffleruby.language.backtrace.BacktraceFormatter;
@@ -119,7 +118,6 @@
119118
import org.truffleruby.language.loader.RequireNode;
120119
import org.truffleruby.language.loader.RequireNodeGen;
121120
import org.truffleruby.language.locals.FindDeclarationVariableNodes.FindAndReadDeclarationVariableNode;
122-
import org.truffleruby.language.methods.DeclarationContext;
123121
import org.truffleruby.language.methods.GetMethodObjectNode;
124122
import org.truffleruby.language.methods.InternalMethod;
125123
import org.truffleruby.language.objects.AllocationTracing;
@@ -1425,104 +1423,71 @@ protected Object send(Frame callerFrame, Object self, Object[] rubyArgs, RootCal
14251423

14261424
}
14271425

1428-
@CoreMethod(names = "respond_to?", required = 1, optional = 1)
1429-
@NodeChild(value = "object", type = RubyNode.class)
1430-
@NodeChild(value = "name", type = RubyNode.class)
1431-
@NodeChild(value = "includeProtectedAndPrivate", type = RubyBaseNodeWithExecute.class)
1432-
public abstract static class RespondToNode extends CoreMethodNode {
1433-
1434-
@Child private InternalRespondToNode dispatch;
1435-
@Child private InternalRespondToNode dispatchIgnoreVisibility;
1436-
@Child private InternalRespondToNode dispatchRespondToMissing;
1437-
@Child private ReadCallerFrameNode readCallerFrame;
1438-
@Child private DispatchNode respondToMissingNode;
1439-
@Child private BooleanCastNode booleanCastNode;
1440-
private final ConditionProfile ignoreVisibilityProfile = ConditionProfile.create();
1441-
private final ConditionProfile isTrueProfile = ConditionProfile.create();
1442-
private final ConditionProfile respondToMissingProfile = ConditionProfile.create();
1443-
1444-
public RespondToNode() {
1445-
dispatch = InternalRespondToNode.create(DispatchConfiguration.PUBLIC);
1446-
dispatchIgnoreVisibility = InternalRespondToNode.create();
1447-
dispatchRespondToMissing = InternalRespondToNode.create();
1448-
}
1449-
1450-
/** Callers should pass null for the frame here, unless they want to use refinements and can ensure the direct
1451-
* caller is a Ruby method */
1452-
public abstract boolean executeDoesRespondTo(VirtualFrame frame, Object object, Object name,
1453-
boolean includeProtectedAndPrivate);
1426+
@ImportStatic(DispatchConfiguration.class)
1427+
@GenerateUncached
1428+
@CoreMethod(names = "respond_to?", required = 1, optional = 1, alwaysInlined = true)
1429+
public abstract static class RespondToNode extends AlwaysInlinedMethodNode {
14541430

1455-
@CreateCast("includeProtectedAndPrivate")
1456-
protected RubyBaseNodeWithExecute coerceToBoolean(RubyBaseNodeWithExecute includeProtectedAndPrivate) {
1457-
return BooleanCastWithDefaultNode.create(false, includeProtectedAndPrivate);
1431+
public final boolean executeDoesRespondTo(Object self, Object name, boolean includeProtectedAndPrivate) {
1432+
final Object[] rubyArgs = RubyArguments.allocate(2);
1433+
RubyArguments.setArgument(rubyArgs, 0, name);
1434+
RubyArguments.setArgument(rubyArgs, 1, includeProtectedAndPrivate);
1435+
return (boolean) execute(null, self, rubyArgs, null);
14581436
}
14591437

14601438
@Specialization
1461-
protected boolean doesRespondTo(
1462-
VirtualFrame frame, Object object, Object name, boolean includeProtectedAndPrivate,
1463-
@Cached ConditionProfile notSymbolOrStringProfile,
1439+
protected boolean doesRespondTo(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget target,
1440+
@Cached BranchProfile notSymbolOrStringProfile,
14641441
@Cached ToJavaStringNode toJavaString,
1465-
@Cached ToSymbolNode toSymbolNode) {
1466-
if (notSymbolOrStringProfile.profile(!RubyGuards.isRubySymbolOrString(name))) {
1442+
@Cached ToSymbolNode toSymbolNode,
1443+
@Cached BooleanCastNode castArgumentNode,
1444+
@Cached ConditionProfile ignoreVisibilityProfile,
1445+
@Cached ConditionProfile isTrueProfile,
1446+
@Cached ConditionProfile respondToMissingProfile,
1447+
@Cached(parameters = "PUBLIC") InternalRespondToNode dispatchPublic,
1448+
@Cached InternalRespondToNode dispatchPrivate,
1449+
@Cached InternalRespondToNode dispatchRespondToMissing,
1450+
@Cached DispatchNode respondToMissingNode,
1451+
@Cached BooleanCastNode castMissingResultNode) {
1452+
final Object name = RubyArguments.getArgument(rubyArgs, 0);
1453+
final int nArgs = RubyArguments.getPositionalArgumentsCount(rubyArgs, false);
1454+
final boolean includeProtectedAndPrivate = nArgs >= 2 &&
1455+
castArgumentNode.executeToBoolean(RubyArguments.getArgument(rubyArgs, 1));
1456+
1457+
if (!RubyGuards.isRubySymbolOrString(name)) {
1458+
notSymbolOrStringProfile.enter();
14671459
throw new RaiseException(
14681460
getContext(),
1469-
coreExceptions().typeErrorIsNotAOrB(object, "symbol", "string", this));
1461+
coreExceptions().typeErrorIsNotAOrB(self, "symbol", "string", this));
14701462
}
14711463

1472-
final boolean ret;
1473-
useCallerRefinements(frame);
1474-
1464+
final String methodName = toJavaString.executeToJavaString(name);
1465+
final boolean found;
14751466
if (ignoreVisibilityProfile.profile(includeProtectedAndPrivate)) {
1476-
ret = dispatchIgnoreVisibility.execute(frame, object, toJavaString.executeToJavaString(name));
1467+
found = dispatchPrivate.execute(callerFrame, self, methodName);
14771468
} else {
1478-
ret = dispatch.execute(frame, object, toJavaString.executeToJavaString(name));
1469+
found = dispatchPublic.execute(callerFrame, self, methodName);
14791470
}
14801471

1481-
if (isTrueProfile.profile(ret)) {
1472+
if (isTrueProfile.profile(found)) {
14821473
return true;
14831474
} else if (respondToMissingProfile
1484-
.profile(dispatchRespondToMissing.execute(frame, object, "respond_to_missing?"))) {
1485-
return respondToMissing(object, toSymbolNode.execute(name), includeProtectedAndPrivate);
1475+
.profile(dispatchRespondToMissing.execute(callerFrame, self, "respond_to_missing?"))) {
1476+
return castMissingResultNode.executeToBoolean(respondToMissingNode.call(self, "respond_to_missing?",
1477+
toSymbolNode.execute(name), includeProtectedAndPrivate));
14861478
} else {
14871479
return false;
14881480
}
14891481
}
1490-
1491-
private boolean respondToMissing(Object object, RubySymbol name, boolean includeProtectedAndPrivate) {
1492-
if (respondToMissingNode == null) {
1493-
CompilerDirectives.transferToInterpreterAndInvalidate();
1494-
respondToMissingNode = insert(DispatchNode.create());
1495-
}
1496-
1497-
if (booleanCastNode == null) {
1498-
CompilerDirectives.transferToInterpreterAndInvalidate();
1499-
booleanCastNode = insert(BooleanCastNode.create());
1500-
}
1501-
1502-
return booleanCastNode.executeToBoolean(
1503-
respondToMissingNode.call(object, "respond_to_missing?", name, includeProtectedAndPrivate));
1504-
}
1505-
1506-
private void useCallerRefinements(VirtualFrame frame) {
1507-
if (frame != null) {
1508-
if (readCallerFrame == null) {
1509-
CompilerDirectives.transferToInterpreterAndInvalidate();
1510-
readCallerFrame = insert(ReadCallerFrameNode.create());
1511-
}
1512-
DeclarationContext context = RubyArguments.getDeclarationContext(readCallerFrame.execute(frame));
1513-
RubyArguments.setDeclarationContext(frame, context);
1514-
}
1515-
}
15161482
}
15171483

1518-
@CoreMethod(names = "respond_to_missing?", required = 2)
1519-
public abstract static class RespondToMissingNode extends CoreMethodArrayArgumentsNode {
1520-
1484+
@GenerateUncached
1485+
@CoreMethod(names = "respond_to_missing?", required = 2, alwaysInlined = true)
1486+
public abstract static class RespondToMissingNode extends AlwaysInlinedMethodNode {
15211487
@Specialization
1522-
protected boolean doesRespondToMissing(Object object, Object name, Object unusedIncludeAll) {
1488+
protected boolean respondToMissing(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget target) {
15231489
return false;
15241490
}
1525-
15261491
}
15271492

15281493
@CoreMethod(names = "set_trace_func", isModuleFunction = true, required = 1)

‎src/main/java/org/truffleruby/core/string/CoreStrings.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ public class CoreStrings {
2828
public final CoreString REPLACEMENT_CHARACTER_SETUP_FAILED;
2929
public final CoreString STACK_LEVEL_TOO_DEEP;
3030
public final CoreString TIME_INTERVAL_MUST_BE_POS;
31-
public final CoreString TO_ARY;
32-
public final CoreString TO_STR;
3331
public final CoreString TOO_FEW_ARGUMENTS;
3432
public final CoreString TZ;
3533
public final CoreString UNKNOWN;
@@ -53,8 +51,6 @@ public CoreStrings(RubyLanguage language) {
5351
REPLACEMENT_CHARACTER_SETUP_FAILED = new CoreString(language, "replacement character setup failed");
5452
STACK_LEVEL_TOO_DEEP = new CoreString(language, "stack level too deep");
5553
TIME_INTERVAL_MUST_BE_POS = new CoreString(language, "time interval must be positive");
56-
TO_ARY = new CoreString(language, "to_ary");
57-
TO_STR = new CoreString(language, "to_str");
5854
TOO_FEW_ARGUMENTS = new CoreString(language, "too few arguments");
5955
TZ = new CoreString(language, "TZ");
6056
UNKNOWN = new CoreString(language, "(unknown)");

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,10 @@ protected boolean equalString(Object a, Object b,
495495
protected boolean equal(Object a, Object b) {
496496
if (respondToNode == null) {
497497
CompilerDirectives.transferToInterpreterAndInvalidate();
498-
respondToNode = insert(KernelNodesFactory.RespondToNodeFactory.create(null, null, null));
498+
respondToNode = insert(KernelNodesFactory.RespondToNodeFactory.create());
499499
}
500500

501-
if (respondToNode.executeDoesRespondTo(null, b, coreStrings().TO_STR.createInstance(getContext()), false)) {
501+
if (respondToNode.executeDoesRespondTo(b, coreSymbols().TO_STR, false)) {
502502
if (objectEqualNode == null) {
503503
CompilerDirectives.transferToInterpreterAndInvalidate();
504504
objectEqualNode = insert(DispatchNode.create());

‎src/main/java/org/truffleruby/core/support/TypeNodes.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.truffleruby.core.encoding.Encodings;
3131
import org.truffleruby.core.kernel.KernelNodes;
3232
import org.truffleruby.core.kernel.KernelNodes.ToSNode;
33-
import org.truffleruby.core.kernel.KernelNodesFactory;
3433
import org.truffleruby.core.klass.RubyClass;
3534
import org.truffleruby.core.module.RubyModule;
3635
import org.truffleruby.core.rope.CodeRange;
@@ -77,16 +76,12 @@ protected boolean objectKindOf(Object object, RubyModule module,
7776

7877
@Primitive(name = "object_respond_to?")
7978
public abstract static class ObjectRespondToNode extends PrimitiveArrayArgumentsNode {
80-
81-
@Child private KernelNodes.RespondToNode respondToNode = KernelNodesFactory.RespondToNodeFactory
82-
.create(null, null, null);
83-
8479
@Specialization
85-
protected boolean objectRespondTo(Object object, Object name, boolean includePrivate) {
80+
protected boolean objectRespondTo(Object object, Object name, boolean includePrivate,
81+
@Cached KernelNodes.RespondToNode respondToNode) {
8682
// Do not pass a frame here, we want to ignore refinements and not need to read the caller frame
87-
return respondToNode.executeDoesRespondTo(null, object, name, includePrivate);
83+
return respondToNode.executeDoesRespondTo(object, name, includePrivate);
8884
}
89-
9085
}
9186

9287
@Primitive(name = "object_class")

‎src/main/java/org/truffleruby/language/arguments/RubyArguments.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,9 @@ public static ArgumentsDescriptor getDescriptor(Frame frame) {
291291
}
292292

293293
public static ArgumentsDescriptor getDescriptor(Object[] args) {
294-
return (ArgumentsDescriptor) args[ArgumentIndicies.DESCRIPTOR.ordinal()];
294+
final Object descriptor = args[ArgumentIndicies.DESCRIPTOR.ordinal()];
295+
assert descriptor instanceof ArgumentsDescriptor : descriptor;
296+
return (ArgumentsDescriptor) descriptor;
295297
}
296298

297299
/** Get the raw number of user arguments inside the frame arguments, which might include keyword arguments */

‎src/main/java/org/truffleruby/language/dispatch/InternalRespondToNode.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010
package org.truffleruby.language.dispatch;
1111

12-
import com.oracle.truffle.api.frame.VirtualFrame;
12+
import com.oracle.truffle.api.frame.Frame;
1313
import com.oracle.truffle.api.nodes.DenyReplace;
1414
import com.oracle.truffle.api.nodes.NodeCost;
1515
import org.truffleruby.core.kernel.KernelNodes.RespondToNode;
@@ -66,7 +66,7 @@ protected InternalRespondToNode(DispatchConfiguration config) {
6666
this(config, MetaClassNode.create(), LookupMethodNode.create());
6767
}
6868

69-
public boolean execute(VirtualFrame frame, Object receiver, String methodName) {
69+
public final boolean execute(Frame frame, Object receiver, String methodName) {
7070
final RubyClass metaclass = metaclassNode.execute(receiver);
7171
final InternalMethod method = methodLookup.execute(frame, metaclass, methodName, config);
7272
return method != null && method.isDefined() && method.isImplemented();
@@ -86,11 +86,6 @@ protected Uncached(DispatchConfiguration config) {
8686
super(config, MetaClassNodeGen.getUncached(), LookupMethodNodeGen.getUncached());
8787
}
8888

89-
@Override
90-
public boolean execute(VirtualFrame frame, Object receiver, String methodName) {
91-
return super.execute(null, receiver, methodName);
92-
}
93-
9489
@Override
9590
public NodeCost getCost() {
9691
return NodeCost.MEGAMORPHIC;

0 commit comments

Comments
 (0)