Skip to content

Commit 99fb4a4

Browse files
eregoneregon
committed
[GR-17173] Fix profiling in ReadCallerFrameNode.
PullRequest: truffleruby/954
2 parents 8fc7e5e + c6cf35b commit 99fb4a4

File tree

12 files changed

+44
-75
lines changed

12 files changed

+44
-75
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Performance:
2929

3030
* Several `String` methods have been made faster by the usage of vector instructions
3131
when searching for a single-byte character in a String.
32+
* Methods needing the caller frame are now better optimized.
3233

3334
# 19.1.0, June 2019
3435

‎src/main/java/org/truffleruby/builtins/CallerFrameAccess.java

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ public abstract static class InstanceEvalNode extends CoreMethodArrayArgumentsNo
266266
public Object instanceEval(VirtualFrame frame, Object receiver, DynamicObject string, DynamicObject fileName, int line, NotProvided block,
267267
@Cached("create()") ReadCallerFrameNode callerFrameNode,
268268
@Cached("create()") IndirectCallNode callNode) {
269-
final MaterializedFrame callerFrame = callerFrameNode.execute(frame).materialize();
269+
final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
270270

271271
return instanceEvalHelper(callerFrame, receiver, string, fileName, line, callNode);
272272
}
@@ -275,7 +275,7 @@ public Object instanceEval(VirtualFrame frame, Object receiver, DynamicObject st
275275
public Object instanceEval(VirtualFrame frame, Object receiver, DynamicObject string, DynamicObject fileName, NotProvided line, NotProvided block,
276276
@Cached("create()") ReadCallerFrameNode callerFrameNode,
277277
@Cached("create()") IndirectCallNode callNode) {
278-
final MaterializedFrame callerFrame = callerFrameNode.execute(frame).materialize();
278+
final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
279279

280280
return instanceEvalHelper(callerFrame, receiver, string, fileName, 1, callNode);
281281
}
@@ -284,7 +284,7 @@ public Object instanceEval(VirtualFrame frame, Object receiver, DynamicObject st
284284
public Object instanceEval(VirtualFrame frame, Object receiver, DynamicObject string, NotProvided fileName, NotProvided line, NotProvided block,
285285
@Cached("create()") ReadCallerFrameNode callerFrameNode,
286286
@Cached("create()") IndirectCallNode callNode) {
287-
final MaterializedFrame callerFrame = callerFrameNode.execute(frame).materialize();
287+
final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
288288

289289
return instanceEvalHelper(callerFrame, receiver, string, coreStrings().EVAL_FILENAME_STRING.createInstance(), 1, callNode);
290290
}

‎src/main/java/org/truffleruby/core/binding/BindingNodes.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import org.jcodings.specific.UTF8Encoding;
1616
import org.truffleruby.Layouts;
1717
import org.truffleruby.RubyContext;
18-
import org.truffleruby.builtins.CallerFrameAccess;
1918
import org.truffleruby.builtins.CoreClass;
2019
import org.truffleruby.builtins.CoreMethod;
2120
import org.truffleruby.builtins.CoreMethodNode;
@@ -388,11 +387,11 @@ public DynamicObject allocate(DynamicObject rubyClass) {
388387
@Primitive(name = "caller_binding", needsSelf = false)
389388
public abstract static class CallerBindingNode extends PrimitiveArrayArgumentsNode {
390389

391-
@Child ReadCallerFrameNode callerFrameNode = new ReadCallerFrameNode(CallerFrameAccess.MATERIALIZE);
390+
@Child ReadCallerFrameNode callerFrameNode = new ReadCallerFrameNode();
392391

393392
@Specialization
394393
public DynamicObject binding(VirtualFrame frame) {
395-
final MaterializedFrame callerFrame = callerFrameNode.execute(frame).materialize();
394+
final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
396395

397396
return BindingNodes.createBinding(getContext(), callerFrame);
398397
}

‎src/main/java/org/truffleruby/core/binding/TruffleBindingNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public DynamicObject ofCaller() {
4242
final MaterializedFrame frame = Truffle.getRuntime().iterateFrames(frameInstance -> {
4343
if (frameCount.get() == 2) {
4444
sourceSection.set(frameInstance.getCallNode().getEncapsulatingSourceSection());
45-
return frameInstance.getFrame(FrameInstance.FrameAccess.READ_WRITE).materialize();
45+
return frameInstance.getFrame(FrameInstance.FrameAccess.MATERIALIZE).materialize();
4646
} else {
4747
frameCount.set(frameCount.get() + 1);
4848
return null;

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.jcodings.specific.UTF8Encoding;
4040
import org.truffleruby.Layouts;
4141
import org.truffleruby.RubyContext;
42-
import org.truffleruby.builtins.CallerFrameAccess;
4342
import org.truffleruby.builtins.CoreClass;
4443
import org.truffleruby.builtins.CoreMethod;
4544
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
@@ -262,11 +261,11 @@ public Object compare(VirtualFrame frame, Object self, Object other) {
262261
@CoreMethod(names = "binding", isModuleFunction = true)
263262
public abstract static class BindingNode extends CoreMethodArrayArgumentsNode {
264263

265-
@Child ReadCallerFrameNode callerFrameNode = new ReadCallerFrameNode(CallerFrameAccess.MATERIALIZE);
264+
@Child ReadCallerFrameNode callerFrameNode = new ReadCallerFrameNode();
266265

267266
@Specialization
268267
protected DynamicObject bindingUncached(VirtualFrame frame) {
269-
final MaterializedFrame callerFrame = callerFrameNode.execute(frame).materialize();
268+
final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
270269
final SourceSection sourceSection = getCallerSourceSection();
271270

272271
return BindingNodes.createBinding(getContext(), callerFrame, sourceSection);
@@ -283,13 +282,13 @@ protected SourceSection getCallerSourceSection() {
283282
@CoreMethod(names = "block_given?", isModuleFunction = true)
284283
public abstract static class BlockGivenNode extends CoreMethodArrayArgumentsNode {
285284

286-
@Child ReadCallerFrameNode callerFrameNode = new ReadCallerFrameNode(CallerFrameAccess.MATERIALIZE);
285+
@Child ReadCallerFrameNode callerFrameNode = new ReadCallerFrameNode();
287286

288287
@Specialization
289288
public boolean blockGiven(VirtualFrame frame,
290289
@Cached("create(nil())") FindAndReadDeclarationVariableNode readNode,
291290
@Cached("createBinaryProfile()") ConditionProfile blockProfile) {
292-
MaterializedFrame callerFrame = callerFrameNode.execute(frame).materialize();
291+
MaterializedFrame callerFrame = callerFrameNode.execute(frame);
293292
return blockProfile.profile(readNode.execute(callerFrame, TranslatorEnvironment.METHOD_BLOCK_NAME) != nil());
294293
}
295294
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ public Object sliceCapture(
631631
@Cached("create()") ReadCallerFrameNode readCallerNode) {
632632
final Object matchStrPair = callNode.call(string, "subpattern", regexp, capture);
633633

634-
final DynamicObject binding = BindingNodes.createBinding(getContext(), readCallerNode.execute(frame).materialize());
634+
final DynamicObject binding = BindingNodes.createBinding(getContext(), readCallerNode.execute(frame));
635635
if (matchStrPair == nil()) {
636636
setLastMatchNode.call(coreLibrary().getTruffleRegexpOperationsModule(), "set_last_match", nil(), binding);
637637
return nil();

‎src/main/java/org/truffleruby/core/thread/TruffleThreadNodes.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import com.oracle.truffle.api.dsl.Cached;
1414
import com.oracle.truffle.api.dsl.ImportStatic;
1515
import com.oracle.truffle.api.dsl.Specialization;
16-
import com.oracle.truffle.api.frame.Frame;
1716
import com.oracle.truffle.api.frame.FrameInstance;
1817
import com.oracle.truffle.api.object.DynamicObject;
1918

@@ -30,8 +29,8 @@ public DynamicObject findRubyCaller(int skip, DynamicObject modules,
3029
@Cached("of(modules)") ArrayStrategy strategy,
3130
@Cached("strategy.boxedCopyNode()") ArrayOperationNodes.ArrayBoxedCopyNode boxedCopyNode) {
3231
Object[] moduleArray = boxedCopyNode.execute(Layouts.ARRAY.getStore(modules), Layouts.ARRAY.getSize(modules));
33-
Frame rubyCaller = getContext().getCallStack().getCallerFrameNotInModules(moduleArray, skip).getFrame(FrameInstance.FrameAccess.MATERIALIZE);
34-
return rubyCaller == null ? nil() : BindingNodes.createBinding(getContext(), rubyCaller.materialize());
32+
FrameInstance rubyCaller = getContext().getCallStack().getCallerFrameNotInModules(moduleArray, skip);
33+
return rubyCaller == null ? nil() : BindingNodes.createBinding(getContext(), rubyCaller.getFrame(FrameInstance.FrameAccess.MATERIALIZE).materialize());
3534
}
3635

3736
}

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
*/
1010
package org.truffleruby.language.arguments;
1111

12-
import org.truffleruby.builtins.CallerFrameAccess;
12+
import com.oracle.truffle.api.CompilerDirectives;
13+
import com.oracle.truffle.api.frame.FrameInstance;
1314
import org.truffleruby.language.RubyBaseNode;
1415
import org.truffleruby.language.dispatch.CachedDispatchNode;
1516

17+
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
1618
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
17-
import com.oracle.truffle.api.frame.Frame;
1819
import com.oracle.truffle.api.frame.MaterializedFrame;
1920
import com.oracle.truffle.api.frame.VirtualFrame;
2021
import com.oracle.truffle.api.nodes.DirectCallNode;
@@ -24,18 +25,21 @@
2425
public class ReadCallerFrameNode extends RubyBaseNode {
2526

2627
private final ConditionProfile callerFrameProfile = ConditionProfile.createBinaryProfile();
27-
28-
private final CallerFrameAccess accessMode;
28+
@CompilationFinal private volatile boolean firstCall = true;
2929

3030
public static ReadCallerFrameNode create() {
31-
return new ReadCallerFrameNode(CallerFrameAccess.MATERIALIZE);
31+
return new ReadCallerFrameNode();
3232
}
3333

34-
public ReadCallerFrameNode(CallerFrameAccess callerFrameAccess) {
35-
this.accessMode = callerFrameAccess;
36-
}
34+
public MaterializedFrame execute(VirtualFrame frame) {
35+
// Avoid polluting the profile for the first call which has to use getCallerFrame()
36+
if (firstCall) {
37+
CompilerDirectives.transferToInterpreterAndInvalidate();
38+
firstCall = false;
39+
notifyCallerToSendFrame();
40+
return getCallerFrame();
41+
}
3742

38-
public Frame execute(VirtualFrame frame) {
3943
final MaterializedFrame callerFrame = RubyArguments.getCallerFrame(frame);
4044

4145
if (callerFrameProfile.profile(callerFrame != null)) {
@@ -45,7 +49,7 @@ public Frame execute(VirtualFrame frame) {
4549
}
4650
}
4751

48-
private void replaceDispatchNode() {
52+
private void notifyCallerToSendFrame() {
4953
final Node callerNode = getContext().getCallStack().getCallerNode(0, false);
5054
if (callerNode instanceof DirectCallNode) {
5155
final Node parent = callerNode.getParent();
@@ -56,9 +60,8 @@ private void replaceDispatchNode() {
5660
}
5761

5862
@TruffleBoundary
59-
private Frame getCallerFrame() {
60-
replaceDispatchNode();
61-
return getContext().getCallStack().getCallerFrameIgnoringSend().getFrame(accessMode.getFrameAccess());
63+
private MaterializedFrame getCallerFrame() {
64+
return getContext().getCallStack().getCallerFrameIgnoringSend().getFrame(FrameInstance.FrameAccess.MATERIALIZE).materialize();
6265
}
6366

6467
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public RunBlockKWArgsHelperNode(FrameSlot arrayFrameSlot, Object kwrestName) {
4040

4141
@Override
4242
public Object execute(VirtualFrame frame) {
43-
notOptimizedWarningNode.warn("keyword arguments are not yet optimized");
43+
notOptimizedWarningNode.warn("keyword rest argument in combination with masgn or destructuring is not yet optimized");
4444

4545
final Object array = readArrayNode.executeRead(frame);
4646

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
package org.truffleruby.language.dispatch;
1111

1212
import org.truffleruby.RubyContext;
13-
import org.truffleruby.builtins.CallerFrameAccess;
1413
import org.truffleruby.core.rope.RopeNodes;
1514
import org.truffleruby.core.string.StringOperations;
1615
import org.truffleruby.language.RubyGuards;
@@ -98,7 +97,7 @@ private synchronized void startSendingFrame(SendsFrame frameToSend) {
9897
assert needsCallerAssumption != AlwaysValidAssumption.INSTANCE;
9998
this.sendsFrame = frameToSend;
10099
if (frameToSend == SendsFrame.CALLER_FRAME) {
101-
this.readCaller = insert(new ReadCallerFrameNode(CallerFrameAccess.MATERIALIZE));
100+
this.readCaller = insert(new ReadCallerFrameNode());
102101
}
103102
Node root = getRootNode();
104103
if (root instanceof RubyRootNode) {
@@ -195,7 +194,7 @@ private MaterializedFrame getFrameIfRequired(VirtualFrame frame) {
195194
case MY_FRAME:
196195
return frame.materialize();
197196
case CALLER_FRAME:
198-
return readCaller.execute(frame).materialize();
197+
return readCaller.execute(frame);
199198
default:
200199
return null;
201200
}

‎src/main/java/org/truffleruby/language/threadlocal/FindThreadAndFrameLocalStorageNode.java

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
1818
import com.oracle.truffle.api.dsl.Cached;
1919
import com.oracle.truffle.api.dsl.Specialization;
20-
import com.oracle.truffle.api.frame.Frame;
2120
import com.oracle.truffle.api.frame.FrameDescriptor;
2221
import com.oracle.truffle.api.frame.FrameSlot;
2322
import com.oracle.truffle.api.frame.FrameSlotKind;
@@ -64,7 +63,7 @@ protected StorageInFrameFinder(FrameDescriptor fd, FrameSlot fs, Object defaultV
6463
this.depth = depth;
6564
}
6665

67-
public boolean matches(Frame callerFrame) {
66+
public boolean matches(MaterializedFrame callerFrame) {
6867
return callerFrame != null && callerFrame.getFrameDescriptor() == descriptor;
6968
}
7069

@@ -73,8 +72,7 @@ public ThreadAndFrameLocalStorage getStorage(RubyContext context, MaterializedFr
7372
return getStorageFromFrame(context, frame, slot, defaultValue, true);
7473
}
7574

76-
public static StorageInFrameFinder of(RubyContext context, Frame aFrame, String variableName) {
77-
MaterializedFrame callerFrame = aFrame.materialize();
75+
public static StorageInFrameFinder of(RubyContext context, MaterializedFrame callerFrame, String variableName) {
7876
FrameDescriptor descriptor = callerFrame.getFrameDescriptor();
7977

8078
int depth = getVariableDeclarationFrameDepth(callerFrame, variableName);
@@ -86,7 +84,7 @@ public static StorageInFrameFinder of(RubyContext context, Frame aFrame, String
8684
}
8785

8886
private static int getVariableDeclarationFrameDepth(MaterializedFrame topFrame, String variableName) {
89-
Frame frame = topFrame;
87+
MaterializedFrame frame = topFrame;
9088
int count = 0;
9189

9290
while (true) {
@@ -95,7 +93,7 @@ private static int getVariableDeclarationFrameDepth(MaterializedFrame topFrame,
9593
return count;
9694
}
9795

98-
final Frame nextFrame = RubyArguments.getDeclarationFrame(frame);
96+
final MaterializedFrame nextFrame = RubyArguments.getDeclarationFrame(frame);
9997
if (nextFrame != null) {
10098
frame = nextFrame;
10199
count++;
@@ -105,16 +103,16 @@ private static int getVariableDeclarationFrameDepth(MaterializedFrame topFrame,
105103
}
106104
}
107105

108-
private static Frame getVariableDeclarationFrame(Frame topFrame, String variableName) {
109-
Frame frame = topFrame;
106+
private static MaterializedFrame getVariableDeclarationFrame(MaterializedFrame topFrame, String variableName) {
107+
MaterializedFrame frame = topFrame;
110108

111109
while (true) {
112110
final FrameSlot slot = getVariableSlot(frame, variableName);
113111
if (slot != null) {
114112
return frame;
115113
}
116114

117-
final Frame nextFrame = RubyArguments.getDeclarationFrame(frame);
115+
final MaterializedFrame nextFrame = RubyArguments.getDeclarationFrame(frame);
118116
if (nextFrame != null) {
119117
frame = nextFrame;
120118
} else {
@@ -123,7 +121,7 @@ private static Frame getVariableDeclarationFrame(Frame topFrame, String variable
123121
}
124122
}
125123

126-
private static FrameSlot getVariableSlot(Frame frame, String variableName) {
124+
private static FrameSlot getVariableSlot(MaterializedFrame frame, String variableName) {
127125
final FrameDescriptor descriptor = frame.getFrameDescriptor();
128126
synchronized (descriptor) {
129127
return descriptor.findFrameSlot(variableName);
@@ -139,16 +137,16 @@ private static FrameSlot getVariableFrameSlotWrite(MaterializedFrame frame, Stri
139137
}
140138

141139
@TruffleBoundary
142-
private static ThreadAndFrameLocalStorage getStorageSearchingDeclarations(RubyContext context, Frame topFrame, String variableName) {
143-
final Frame frame = getVariableDeclarationFrame(topFrame, variableName);
140+
private static ThreadAndFrameLocalStorage getStorageSearchingDeclarations(RubyContext context, MaterializedFrame topFrame, String variableName) {
141+
final MaterializedFrame frame = getVariableDeclarationFrame(topFrame, variableName);
144142
if (frame == null) {
145143
return null;
146144
}
147145
FrameSlot slot = getVariableFrameSlotWrite(frame.materialize(), variableName);
148146
return getStorageFromFrame(context, frame, slot, frame.getFrameDescriptor().getDefaultValue(), true);
149147
}
150148

151-
private static ThreadAndFrameLocalStorage getStorageFromFrame(RubyContext context, Frame frame, FrameSlot slot, Object defaultValue, boolean add) {
149+
private static ThreadAndFrameLocalStorage getStorageFromFrame(RubyContext context, MaterializedFrame frame, FrameSlot slot, Object defaultValue, boolean add) {
152150
final Object previousMatchData = frame.getValue(slot);
153151

154152
if (previousMatchData == defaultValue) { // Never written to

0 commit comments

Comments
 (0)