Skip to content

Commit 655ecb9

Browse files
author
Pavel Marek
committed
Performance improvements.
1 parent ddfa3ff commit 655ecb9

File tree

10 files changed

+81
-38
lines changed

10 files changed

+81
-38
lines changed

‎com.oracle.truffle.r.library/src/com/oracle/truffle/r/library/stats/deriv/Deriv.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
import com.oracle.truffle.r.runtime.nodes.RSyntaxLookup;
7878
import com.oracle.truffle.r.runtime.nodes.RSyntaxNode;
7979
import com.oracle.truffle.r.runtime.nodes.RSyntaxVisitor;
80+
import com.oracle.truffle.r.runtime.parsermetadata.FunctionScope;
8081

8182
//Transcribed from GnuR, library/stats/src/deriv.c
8283

@@ -190,7 +191,7 @@ private Object getResult(TruffleRLanguage language, Object functionArg) {
190191
return result;
191192
}
192193
MaterializedFrame frame = functionArg instanceof RFunction ? ((RFunction) functionArg).getEnclosingFrame() : RContext.getInstance().stateREnvironment.getGlobalFrame();
193-
RootCallTarget callTarget = RContext.getASTBuilder().rootFunction(language, RSyntaxNode.LAZY_DEPARSE, targetArgs, blockCall, null, null);
194+
RootCallTarget callTarget = RContext.getASTBuilder().rootFunction(language, RSyntaxNode.LAZY_DEPARSE, targetArgs, blockCall, null, FunctionScope.EMPTY_SCOPE);
194195
FrameSlotChangeMonitor.initializeEnclosingFrame(callTarget.getRootNode().getFrameDescriptor(), frame);
195196
return RDataFactory.createFunction(RFunction.NO_NAME, RFunction.NO_NAME, callTarget, null, frame);
196197
}

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/FunctionBuiltin.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.oracle.truffle.r.runtime.context.TruffleRLanguage;
4141
import com.oracle.truffle.r.runtime.nodes.RCodeBuilder.Argument;
4242
import com.oracle.truffle.r.runtime.nodes.RSyntaxNode;
43+
import com.oracle.truffle.r.runtime.parsermetadata.FunctionScope;
4344

4445
@RBuiltin(name = "function", kind = PRIMITIVE, nonEvalArgs = 1, parameterNames = {"args", "body"}, behavior = COMPLEX)
4546
public final class FunctionBuiltin extends RBuiltinNode.Arg2 {
@@ -76,6 +77,7 @@ public Object execute(MaterializedFrame frame, TruffleRLanguage language, Object
7677
public static FunctionExpressionNode createFunctionExpressionNode(TruffleRLanguage language, Object args, Object body) {
7778
List<Argument<RSyntaxNode>> finalArgs = RContext.getASTBuilder().getFunctionExprArgs(args);
7879
// TODO: Search for local variables?
79-
return (FunctionExpressionNode) RContext.getASTBuilder().function(language, RSyntaxNode.LAZY_DEPARSE, finalArgs, RASTUtils.createNodeForValue(body).asRSyntaxNode(), null, null);
80+
return (FunctionExpressionNode) RContext.getASTBuilder().function(language, RSyntaxNode.LAZY_DEPARSE, finalArgs, RASTUtils.createNodeForValue(body).asRSyntaxNode(), null,
81+
FunctionScope.EMPTY_SCOPE);
8082
}
8183
}

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RASTBuilder.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.oracle.truffle.api.RootCallTarget;
3333
import com.oracle.truffle.api.TruffleLogger;
3434
import com.oracle.truffle.api.frame.FrameDescriptor;
35-
import com.oracle.truffle.api.frame.FrameSlotKind;
3635
import com.oracle.truffle.api.object.DynamicObject;
3736
import com.oracle.truffle.api.source.SourceSection;
3837
import com.oracle.truffle.r.nodes.access.AccessArgumentNode;
@@ -89,7 +88,7 @@
8988
*/
9089
public final class RASTBuilder implements RCodeBuilder<RSyntaxNode> {
9190

92-
private static TruffleLogger logger = RLogger.getLogger(RASTBuilder.class.getSimpleName());
91+
private static final TruffleLogger logger = RLogger.getLogger(RLogger.LOGGER_AST);
9392
private CodeBuilderContext context = CodeBuilderContext.DEFAULT;
9493
private ParseDataBuilder parseDataBuilder;
9594

@@ -297,6 +296,7 @@ public ArrayList<Argument<RSyntaxNode>> getFunctionExprArgs(Object args) {
297296

298297
@Override
299298
public RootCallTarget rootFunction(TruffleRLanguage language, SourceSection source, List<Argument<RSyntaxNode>> params, RSyntaxNode body, String name, FunctionScope functionScope) {
299+
assert functionScope != null;
300300
// Parse argument list
301301
logger.finer("rootFunction '" + name + "'");
302302
RNode[] defaultValues = new RNode[params.size()];
@@ -343,11 +343,9 @@ public RootCallTarget rootFunction(TruffleRLanguage language, SourceSection sour
343343
}
344344

345345
// Local variables
346-
logger.fine(() -> String.format("rootFunction('%s'): functionScope = %s", name, functionScope != null ? functionScope.toString() : "null"));
347-
List<?> identifiers = (functionScope != null) ? functionScope.getLocalVariableNames() : new ArrayList<>();
348-
List<FrameSlotKind> slotKinds = (functionScope != null) ? functionScope.getLocalVariableKinds() : new ArrayList<>();
346+
logger.fine(() -> String.format("rootFunction('%s'): functionScope = %s", name, functionScope != FunctionScope.EMPTY_SCOPE ? functionScope.toString() : "empty scope"));
349347
String functionFrameDescriptorName = name != null && !name.isEmpty() ? name : "<function>";
350-
FrameDescriptor descriptor = FrameSlotChangeMonitor.createFunctionFrameDescriptor(functionFrameDescriptorName, slotKinds, identifiers);
348+
FrameDescriptor descriptor = FrameSlotChangeMonitor.createFunctionFrameDescriptor(functionFrameDescriptorName, functionScope);
351349
FunctionDefinitionNode rootNode = FunctionDefinitionNode.create(language, source, descriptor, argSourceSections, saveArguments, body, formals, name, argPostProcess);
352350

353351
if (RContext.getInstance().getOption(ForceSources)) {

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/variables/ReadVariableNode.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,11 +588,11 @@ private void initializeFrameIndexAndAssumption(FrameDescriptor variableFrameDesc
588588

589589
@TruffleBoundary
590590
private Object getValue(MaterializedFrame current) {
591-
int frameIndex = FrameSlotChangeMonitor.getIndexOfIdentifier(current.getFrameDescriptor(), identifier);
592-
if (FrameIndex.isUninitializedIndex(frameIndex)) {
591+
int identifierFrameIndex = FrameSlotChangeMonitor.getIndexOfIdentifier(current.getFrameDescriptor(), identifier);
592+
if (FrameIndex.isUninitializedIndex(identifierFrameIndex)) {
593593
return null;
594594
} else {
595-
return FrameSlotChangeMonitor.getValue(current, frameIndex);
595+
return FrameSlotChangeMonitor.getValue(current, identifierFrameIndex);
596596
}
597597
}
598598

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
import com.oracle.truffle.r.runtime.nodes.RSyntaxLookup;
8888
import com.oracle.truffle.r.runtime.nodes.RSyntaxNode;
8989
import com.oracle.truffle.r.runtime.nodes.RSyntaxVisitor;
90+
import com.oracle.truffle.r.runtime.parsermetadata.FunctionScope;
9091

9192
public final class FunctionDefinitionNode extends RRootNode implements RSyntaxNode, RSyntaxFunction {
9293

@@ -192,7 +193,7 @@ public RootCallTarget duplicateWithNewFrameDescriptor() {
192193
SourceSection source = argSourceSections == null ? getLazySourceSection() : argSourceSections[i];
193194
args.add(RCodeBuilder.argument(source, getFormalArguments().getSignature().getName(i), value == null ? null : builder.process(value.asRSyntaxNode())));
194195
}
195-
RootCallTarget callTarget = builder.rootFunction(getRLanguage(), getLazySourceSection(), args, builder.process(getBody()), name, null);
196+
RootCallTarget callTarget = builder.rootFunction(getRLanguage(), getLazySourceSection(), args, builder.process(getBody()), name, FunctionScope.EMPTY_SCOPE);
196197
return callTarget;
197198
}
198199

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RLogger.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ public class RLogger {
115115

116116
public static final String LOGGER_FRAMES = "com.oracle.truffle.r.frames";
117117

118+
public static final String LOGGER_AST = "com.oracle.truffle.r.ast";
119+
118120
public static TruffleLogger getLogger(String name) {
119121
return TruffleLogger.getLogger(R_LANGUAGE_ID, name);
120122
}

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSubstitute.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.oracle.truffle.r.runtime.nodes.RSyntaxLookup;
4949
import com.oracle.truffle.r.runtime.nodes.RSyntaxNode;
5050
import com.oracle.truffle.r.runtime.nodes.RSyntaxVisitor;
51+
import com.oracle.truffle.r.runtime.parsermetadata.FunctionScope;
5152

5253
public class RSubstitute {
5354

@@ -210,7 +211,7 @@ protected T visit(RSyntaxLookup element) {
210211
protected T visit(RSyntaxFunction element) {
211212
ArrayList<Argument<T>> params = createArguments(element.getSyntaxSignature(), element.getSyntaxArgumentDefaults());
212213
// TODO: Search for localVariables
213-
return builder.function(language, RSyntaxNode.LAZY_DEPARSE, params, accept(element.getSyntaxBody()), element.getSyntaxDebugName(), null);
214+
return builder.function(language, RSyntaxNode.LAZY_DEPARSE, params, accept(element.getSyntaxBody()), element.getSyntaxDebugName(), FunctionScope.EMPTY_SCOPE);
214215
}
215216
}.accept(original);
216217
}

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/env/frame/FrameSlotChangeMonitor.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
import com.oracle.truffle.r.runtime.data.RPromise;
7070
import com.oracle.truffle.r.runtime.data.RSharingAttributeStorage;
7171
import com.oracle.truffle.r.runtime.data.RUnboundValue;
72+
import com.oracle.truffle.r.runtime.parsermetadata.FunctionScope;
7273

7374
/**
7475
* This class handles all the accesses and manipulations with {@link FrameDescriptor frame
@@ -569,18 +570,17 @@ public static FrameDescriptor createFunctionFrameDescriptor(String name) {
569570
* Creates a {@link FrameDescriptor} with normal indexed slots.
570571
*
571572
* @param name Name for debug purposes.
572-
* @param kinds Kind of normal slots to be initialized in the frame descriptor.
573-
* @param identifiers Identifiers of normal slots
574573
*/
575-
public static FrameDescriptor createFunctionFrameDescriptor(String name, List<FrameSlotKind> kinds, List<?> identifiers) {
576-
assert kinds.size() == identifiers.size();
577-
Builder builder = FrameDescriptor.newBuilder();
574+
public static FrameDescriptor createFunctionFrameDescriptor(String name, FunctionScope functionScope) {
575+
int localVariableCount = functionScope.getLocalVariableCount();
576+
Builder builder = FrameDescriptor.newBuilder(INTERNAL_INDEXED_SLOT_COUNT + localVariableCount);
578577
var descriptorMetadata = new FrameDescriptorMetaData(name);
579578
builder.info(descriptorMetadata);
580579
addInternalIndexedSlots(builder, descriptorMetadata);
581-
for (int i = 0; i < kinds.size(); i++) {
582-
int frameIndex = builder.addSlot(kinds.get(i), identifiers.get(i), new FrameSlotInfo(descriptorMetadata, identifiers.get(i)));
583-
descriptorMetadata.addIndex(identifiers.get(i), FrameIndex.toNormalIndex(frameIndex));
580+
for (int i = 0; i < localVariableCount; i++) {
581+
String identifier = functionScope.getLocalVariableName(i);
582+
int frameIndex = builder.addSlot(functionScope.getLocalVariableKind(i), identifier, new FrameSlotInfo(descriptorMetadata, identifier));
583+
descriptorMetadata.addIndex(identifier, FrameIndex.toNormalIndex(frameIndex));
584584
}
585585
FrameDescriptor frameDescriptor = builder.build();
586586
assert assertValidFrameDescriptor(frameDescriptor);

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/nodes/RCodeBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ protected T visit(RSyntaxLookup element) {
209209
@Override
210210
protected T visit(RSyntaxFunction element) {
211211
ArrayList<Argument<T>> params = createArguments(element.getSyntaxSignature(), element.getSyntaxArgumentDefaults());
212-
return function(RContext.getInstance().getLanguage(), element.getLazySourceSection(), params, accept(element.getSyntaxBody()), element.getSyntaxDebugName(), null);
212+
return function(RContext.getInstance().getLanguage(), element.getLazySourceSection(), params, accept(element.getSyntaxBody()), element.getSyntaxDebugName(), FunctionScope.EMPTY_SCOPE);
213213
}
214214
}.accept(original);
215215
if (result instanceof Node) {

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/parsermetadata/FunctionScope.java‎

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,28 @@
2828
import java.util.List;
2929
import java.util.Map;
3030

31+
import com.oracle.truffle.api.TruffleLogger;
3132
import com.oracle.truffle.api.frame.FrameSlotKind;
3233
import com.oracle.truffle.r.runtime.RLogger;
3334
import com.oracle.truffle.r.runtime.env.frame.FrameSlotChangeMonitor;
3435

3536
public final class FunctionScope {
3637
private String functionName;
37-
private int localVarFrameIdx = FrameSlotChangeMonitor.INTERNAL_INDEXED_SLOT_COUNT;
38+
39+
public static final FunctionScope EMPTY_SCOPE = new FunctionScope();
3840

3941
private static final int INITIAL_LOC_VARS_CAPACITY = 12;
4042

4143
private final List<FrameSlotKind> localVariableKinds = new ArrayList<>(INITIAL_LOC_VARS_CAPACITY);
4244
private final List<String> localVariableNames = new ArrayList<>(INITIAL_LOC_VARS_CAPACITY);
43-
private final Map<String, Integer> localVariableIndexes = new HashMap<>(INITIAL_LOC_VARS_CAPACITY);
45+
46+
/**
47+
* Maps identifiers to indexes into lists used in this class. Note that these indexes are not
48+
* {@link com.oracle.truffle.r.runtime.env.frame.FrameIndex frame indexes}.
49+
*/
50+
private final Map<String, Integer> localVariableArrayIndexes = new HashMap<>(INITIAL_LOC_VARS_CAPACITY);
51+
52+
private static final TruffleLogger logger = RLogger.getLogger(RLogger.LOGGER_AST);
4453

4554
public FunctionScope() {
4655
this(null);
@@ -58,30 +67,59 @@ public String getFunctionName() {
5867
return functionName;
5968
}
6069

61-
public boolean containsLocalVariable(String name) {
62-
return localVariableIndexes.containsKey(name);
63-
}
64-
6570
public void addLocalVariable(String identifier, FrameSlotKind slotKind) {
66-
if (!containsLocalVariable(identifier)) {
67-
RLogger.getLogger("RASTBuilder").fine(() -> String.format("Adding local variable %s to function scope '%s'", identifier, functionName));
68-
localVariableIndexes.put(identifier, localVarFrameIdx);
69-
localVariableKinds.add(slotKind);
71+
Integer arrayIdx = localVariableArrayIndexes.get(identifier);
72+
if (arrayIdx == null) {
73+
logger.fine(() -> String.format("Adding local variable %s to function scope '%s'", identifier, functionName));
74+
localVariableArrayIndexes.put(identifier, localVariableNames.size());
7075
localVariableNames.add(identifier);
71-
localVarFrameIdx++;
76+
localVariableKinds.add(slotKind);
77+
} else {
78+
// Because of the implementation of FrameIndex, frameIndex can also index an array.
79+
// In this case, the local variable is redefined, so we replace its old slot kind with
80+
// the newly defined slot kind.
81+
FrameSlotKind oldKind = localVariableKinds.get(arrayIdx);
82+
localVariableKinds.set(arrayIdx, replaceKind(oldKind, slotKind));
83+
}
84+
}
85+
86+
/**
87+
* Returns a frame slot kind that should replace the given {@code oldKind} by {@code newKind}.
88+
* For example {@code replaceKind(Integer, Double) => Object}, or
89+
* {@code replaceKind(Illegal, Object) => Object}, etc.
90+
*/
91+
private static FrameSlotKind replaceKind(FrameSlotKind oldKind, FrameSlotKind newKind) {
92+
// TODO: Add if clause that returns Illegal.
93+
if (oldKind == newKind) {
94+
return oldKind;
95+
} else {
96+
return FrameSlotKind.Object;
7297
}
7398
}
7499

100+
public int getLocalVariableCount() {
101+
return localVariableNames.size();
102+
}
103+
104+
/**
105+
* Returns {@link com.oracle.truffle.r.runtime.env.frame.FrameIndex frame index} of the given
106+
* local variable.
107+
*
108+
* Note that this method uses knowledge of internal implementation of
109+
* {@link com.oracle.truffle.r.runtime.env.frame.FrameIndex} - indexes into normal slots (not
110+
* auxiliary) are 0-based positive integers.
111+
*/
75112
public Integer getLocalVariableFrameIndex(String symbol) {
76-
return localVariableIndexes.get(symbol);
113+
Integer locVarArrayIdx = localVariableArrayIndexes.get(symbol);
114+
return locVarArrayIdx != null ? locVarArrayIdx + FrameSlotChangeMonitor.INTERNAL_INDEXED_SLOT_COUNT : null;
77115
}
78116

79-
public List<String> getLocalVariableNames() {
80-
return localVariableNames;
117+
public String getLocalVariableName(int locVarIdx) {
118+
return localVariableNames.get(locVarIdx);
81119
}
82120

83-
public List<FrameSlotKind> getLocalVariableKinds() {
84-
return localVariableKinds;
121+
public FrameSlotKind getLocalVariableKind(int locVarIdx) {
122+
return localVariableKinds.get(locVarIdx);
85123
}
86124

87125
@Override

0 commit comments

Comments
 (0)