Skip to content

Commit adea47f

Browse files
eregoneregon
committed
Fix the Symbol#to_proc cache to be correct with AST sharing
* We must not store context-specific data like refinements or RubyProc in a ImmutableRubyObject like RubySymbol.
1 parent 00439ff commit adea47f

File tree

6 files changed

+82
-56
lines changed

6 files changed

+82
-56
lines changed

‎src/main/java/org/truffleruby/RubyContext.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
import java.util.Objects;
1818
import java.util.Map;
1919
import java.util.WeakHashMap;
20+
import java.util.concurrent.ConcurrentHashMap;
2021
import java.util.concurrent.locks.ReentrantLock;
2122
import java.util.logging.Level;
2223

2324
import com.oracle.truffle.api.CompilerAsserts;
2425
import com.oracle.truffle.api.CompilerDirectives;
26+
import com.oracle.truffle.api.RootCallTarget;
2527
import com.oracle.truffle.api.nodes.EncapsulatingNodeReference;
2628
import com.oracle.truffle.api.nodes.Node;
29+
import org.graalvm.collections.Pair;
2730
import org.graalvm.options.OptionDescriptor;
2831
import org.joni.Regex;
2932
import org.truffleruby.cext.ValueWrapperManager;
@@ -43,6 +46,7 @@
4346
import org.truffleruby.core.kernel.AtExitManager;
4447
import org.truffleruby.core.kernel.TraceManager;
4548
import org.truffleruby.core.module.ModuleOperations;
49+
import org.truffleruby.core.module.RubyModule;
4650
import org.truffleruby.core.objectspace.ObjectSpaceManager;
4751
import org.truffleruby.core.proc.ProcOperations;
4852
import org.truffleruby.core.proc.RubyProc;
@@ -51,6 +55,7 @@
5155
import org.truffleruby.core.rope.Rope;
5256
import org.truffleruby.core.string.FrozenStringLiterals;
5357
import org.truffleruby.core.string.RubyString;
58+
import org.truffleruby.core.symbol.RubySymbol;
5459
import org.truffleruby.core.thread.ThreadManager;
5560
import org.truffleruby.core.time.GetTimeZoneNode;
5661
import org.truffleruby.debug.MetricsProfiler;
@@ -121,6 +126,8 @@ public class RubyContext {
121126
private final NativeConfiguration nativeConfiguration;
122127
private final ValueWrapperManager valueWrapperManager;
123128
private final Map<Source, Integer> sourceLineOffsets = Collections.synchronizedMap(new WeakHashMap<>());
129+
/** (Symbol, refinements) -> Proc for Symbol#to_proc */
130+
public final Map<Pair<RubySymbol, Map<RubyModule, RubyModule[]>>, RootCallTarget> cachedSymbolToProcTargetsWithRefinements = new ConcurrentHashMap<>();
124131

125132
@CompilationFinal private SecureRandom random;
126133
private final Hashing hashing;

‎src/main/java/org/truffleruby/core/cast/ToProcNode.java

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

12+
import com.oracle.truffle.api.RootCallTarget;
1213
import org.truffleruby.core.module.RubyModule;
1314
import org.truffleruby.core.proc.RubyProc;
1415
import org.truffleruby.core.symbol.RubySymbol;
@@ -46,7 +47,7 @@ protected RubyProc doRubyProc(RubyProc proc) {
4647
// No need to guard the refinements here since refinements are always the same in a given source location
4748
@Specialization(
4849
guards = "symbol == cachedSymbol",
49-
assumptions = "getContext().getLanguageSlow().coreMethodAssumptions.symbolToProcAssumption",
50+
assumptions = "getLanguage().coreMethodAssumptions.symbolToProcAssumption",
5051
limit = "1")
5152
protected Object doRubySymbolASTInlined(VirtualFrame frame, RubySymbol symbol,
5253
@Cached("symbol") RubySymbol cachedSymbol,
@@ -84,7 +85,9 @@ protected RubyProc doObject(VirtualFrame frame, Object object,
8485
}
8586

8687
protected RubyProc getProcForSymbol(Map<RubyModule, RubyModule[]> refinements, RubySymbol symbol) {
87-
return SymbolNodes.ToProcNode.getOrCreateProc(getContext(), getLanguage(), refinements, symbol);
88+
final RootCallTarget callTarget = SymbolNodes.ToProcNode
89+
.getOrCreateCallTarget(getContext(), getLanguage(), symbol, refinements);
90+
return SymbolNodes.ToProcNode.createProc(getContext(), getLanguage(), refinements, callTarget);
8891
}
8992

9093
protected static Map<RubyModule, RubyModule[]> getRefinements(VirtualFrame frame) {

‎src/main/java/org/truffleruby/core/symbol/RubySymbol.java

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,11 @@
1010
package org.truffleruby.core.symbol;
1111

1212
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
13+
import com.oracle.truffle.api.RootCallTarget;
1314
import org.truffleruby.RubyContext;
1415
import org.truffleruby.RubyLanguage;
1516
import org.truffleruby.core.Hashing;
1617
import org.truffleruby.core.klass.RubyClass;
17-
import org.truffleruby.core.module.RubyModule;
18-
import org.truffleruby.core.proc.RubyProc;
1918
import org.truffleruby.core.rope.Rope;
2019
import org.truffleruby.language.ImmutableRubyObject;
2120

@@ -26,10 +25,7 @@
2625
import com.oracle.truffle.api.interop.TruffleObject;
2726
import com.oracle.truffle.api.library.ExportLibrary;
2827
import com.oracle.truffle.api.library.ExportMessage;
29-
30-
import java.util.Map;
31-
import java.util.concurrent.ConcurrentHashMap;
32-
import java.util.concurrent.ConcurrentMap;
28+
import org.truffleruby.language.methods.DeclarationContext;
3329

3430
@ExportLibrary(InteropLibrary.class)
3531
public final class RubySymbol extends ImmutableRubyObject implements TruffleObject {
@@ -42,8 +38,8 @@ public final class RubySymbol extends ImmutableRubyObject implements TruffleObje
4238
private final Rope rope;
4339
private final int javaStringHashCode;
4440
private final long id;
45-
/** refinements -> Proc for Symbol#to_proc */
46-
private final ConcurrentMap<Map<RubyModule, RubyModule[]>, RubyProc> cachedProcs = new ConcurrentHashMap<>();
41+
42+
private volatile RootCallTarget callTargetNoRefinements = null;
4743

4844
public RubySymbol(String string, Rope rope, long id) {
4945
this.string = string;
@@ -68,8 +64,18 @@ public Rope getRope() {
6864
return rope;
6965
}
7066

71-
public ConcurrentMap<Map<RubyModule, RubyModule[]>, RubyProc> getCachedProcs() {
72-
return cachedProcs;
67+
@TruffleBoundary
68+
public RootCallTarget getCallTargetNoRefinements(RubyLanguage language) {
69+
if (callTargetNoRefinements == null) {
70+
synchronized (this) {
71+
if (callTargetNoRefinements == null) {
72+
callTargetNoRefinements = SymbolNodes.ToProcNode
73+
.createCallTarget(language, this, DeclarationContext.NO_REFINEMENTS);
74+
}
75+
}
76+
}
77+
78+
return callTargetNoRefinements;
7379
}
7480

7581
public long computeHashCode(Hashing hashing) {

‎src/main/java/org/truffleruby/core/symbol/SymbolNodes.java

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010
package org.truffleruby.core.symbol;
1111

12+
import org.graalvm.collections.Pair;
1213
import org.truffleruby.RubyContext;
1314
import org.truffleruby.RubyLanguage;
1415
import org.truffleruby.builtins.CoreMethod;
@@ -25,6 +26,7 @@
2526
import org.truffleruby.core.proc.RubyProc;
2627
import org.truffleruby.core.string.RubyString;
2728
import org.truffleruby.core.string.StringNodes;
29+
import org.truffleruby.language.LexicalScope;
2830
import org.truffleruby.language.RubyRootNode;
2931
import org.truffleruby.language.Visibility;
3032
import org.truffleruby.language.arguments.ReadCallerFrameNode;
@@ -122,74 +124,56 @@ public static ToProcNode create() {
122124
protected RubyProc toProcCached(VirtualFrame frame, RubySymbol symbol,
123125
@Cached("symbol") RubySymbol cachedSymbol,
124126
@Cached("getRefinements(frame)") Map<RubyModule, RubyModule[]> cachedRefinements,
125-
@Cached("getOrCreateProc(getContext(), getLanguage(), cachedRefinements, symbol)") RubyProc cachedProc) {
127+
@Cached("getOrCreateCallTarget(getContext(), getLanguage(), cachedSymbol, cachedRefinements)") RootCallTarget callTarget,
128+
@Cached("createProc(getContext(), getLanguage(), cachedRefinements, callTarget)") RubyProc cachedProc) {
126129
return cachedProc;
127130
}
128131

129132
@Specialization(replaces = "toProcCached")
130133
protected RubyProc toProcUncached(VirtualFrame frame, RubySymbol symbol) {
131134
final Map<RubyModule, RubyModule[]> refinements = getRefinements(frame);
132-
return getOrCreateProc(getContext(), getLanguage(), refinements, symbol);
135+
final RootCallTarget callTarget = getOrCreateCallTarget(getContext(), getLanguage(), symbol, refinements);
136+
return createProc(getContext(), getLanguage(), refinements, callTarget);
133137
}
134138

135139
@TruffleBoundary
136-
public static RubyProc getOrCreateProc(
137-
RubyContext context,
138-
RubyLanguage language,
139-
Map<RubyModule, RubyModule[]> refinements,
140-
RubySymbol symbol) {
141-
// TODO (eregon, 23 Sep 2020): this should ideally cache on the refinements by comparing classes, and not by identity.
142-
return ConcurrentOperations.getOrCompute(
143-
symbol.getCachedProcs(),
144-
refinements,
145-
key -> createProc(context, language, key, symbol));
146-
}
147-
148-
@TruffleBoundary
149-
private static RubyProc createProc(RubyContext context, RubyLanguage language,
150-
Map<RubyModule, RubyModule[]> refinements,
151-
RubySymbol symbol) {
140+
public static RootCallTarget getOrCreateCallTarget(RubyContext context, RubyLanguage language,
141+
RubySymbol symbol, Map<RubyModule, RubyModule[]> refinements) {
142+
if (refinements == DeclarationContext.NO_REFINEMENTS) {
143+
return symbol.getCallTargetNoRefinements(language);
144+
} else {
145+
// TODO (eregon, 23 Sep 2020): this should ideally cache on the refinements by comparing classes, and not by identity.
146+
return ConcurrentOperations.getOrCompute(
147+
context.cachedSymbolToProcTargetsWithRefinements,
148+
Pair.create(symbol, refinements),
149+
key -> createCallTarget(language, symbol, refinements));
150+
}
151+
}
152+
153+
public static RubyProc createProc(RubyContext context, RubyLanguage language,
154+
Map<RubyModule, RubyModule[]> refinements, RootCallTarget callTarget) {
152155
final InternalMethod method = context.getCoreMethods().SYMBOL_TO_PROC;
153-
final SourceSection sourceSection = CoreLibrary.UNAVAILABLE_SOURCE_SECTION;
154-
final DeclarationContext declarationContext = refinements.isEmpty()
156+
final DeclarationContext declarationContext = refinements == DeclarationContext.NO_REFINEMENTS
155157
? DeclarationContext.NONE
156158
: new DeclarationContext(Visibility.PUBLIC, null, refinements);
157159

158-
final SharedMethodInfo sharedMethodInfo = new SharedMethodInfo(
159-
sourceSection,
160-
method.getLexicalScope(),
161-
ARITY,
162-
null,
163-
symbol.getString(),
164-
0,
165-
"proc",
166-
ArgumentDescriptor.ANON_REST);
167160
final Object[] args = RubyArguments
168161
.pack(null, null, method, declarationContext, null, nil, null, EMPTY_ARGUMENTS);
169-
// MRI raises an error on Proc#binding if you attempt to access the binding of a procedure generated
162+
// MRI raises an error on Proc#binding if you attempt to access the binding of a Proc generated
170163
// by Symbol#to_proc. We generate a declaration frame here so that all procedures will have a
171164
// binding as this simplifies the logic elsewhere in the runtime.
172165
final MaterializedFrame declarationFrame = Truffle
173166
.getRuntime()
174-
.createMaterializedFrame(args, context.getCoreLibrary().emptyDeclarationDescriptor);
167+
.createVirtualFrame(args, context.getCoreLibrary().emptyDeclarationDescriptor)
168+
.materialize();
175169
SpecialVariableStorage variables = new SpecialVariableStorage();
176170
declarationFrame.setObject(context.getCoreLibrary().emptyDeclarationSpecialVariableSlot, variables);
177171

178-
final RubyRootNode rootNode = new RubyRootNode(
179-
language,
180-
sourceSection,
181-
new FrameDescriptor(nil),
182-
sharedMethodInfo,
183-
new SymbolProcNode(symbol.getString()),
184-
Split.HEURISTIC);
185-
186-
final RootCallTarget callTarget = Truffle.getRuntime().createCallTarget(rootNode);
187-
188172
return ProcOperations.createRubyProc(
189173
context.getCoreLibrary().procClass,
190174
language.procShape,
191175
ProcType.PROC,
192-
sharedMethodInfo,
176+
((RubyRootNode) callTarget.getRootNode()).getSharedMethodInfo(),
193177
callTarget,
194178
callTarget,
195179
declarationFrame,
@@ -200,6 +184,32 @@ private static RubyProc createProc(RubyContext context, RubyLanguage language,
200184
declarationContext);
201185
}
202186

187+
public static RootCallTarget createCallTarget(RubyLanguage language, RubySymbol symbol,
188+
// unused but the CallTarget will capture the refinements in the DispatchNode on first call
189+
Map<RubyModule, RubyModule[]> refinements) {
190+
final SourceSection sourceSection = CoreLibrary.UNAVAILABLE_SOURCE_SECTION;
191+
192+
final SharedMethodInfo sharedMethodInfo = new SharedMethodInfo(
193+
sourceSection,
194+
LexicalScope.IGNORE,
195+
ARITY,
196+
null,
197+
symbol.getString(),
198+
0,
199+
"proc",
200+
ArgumentDescriptor.ANON_REST);
201+
202+
final RubyRootNode rootNode = new RubyRootNode(
203+
language,
204+
sourceSection,
205+
new FrameDescriptor(nil),
206+
sharedMethodInfo,
207+
new SymbolProcNode(symbol.getString()),
208+
Split.HEURISTIC);
209+
210+
return Truffle.getRuntime().createCallTarget(rootNode);
211+
}
212+
203213
protected InternalMethod getMethod(VirtualFrame frame) {
204214
return RubyArguments.getMethod(frame);
205215
}

‎src/main/java/org/truffleruby/language/RubyRootNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import com.oracle.truffle.api.utilities.CyclicAssumption;
2121
import org.truffleruby.language.methods.Split;
2222

23-
public class RubyRootNode extends RubyBaseRootNode {
23+
public final class RubyRootNode extends RubyBaseRootNode {
2424

2525
private final RubyLanguage language;
2626
private final SharedMethodInfo sharedMethodInfo;

‎src/main/java/org/truffleruby/language/methods/DeclarationContext.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
* </ul>
3232
*/
3333
public class DeclarationContext {
34-
private static final Map<RubyModule, RubyModule[]> NO_REFINEMENTS = Collections.emptyMap();
34+
public static final Map<RubyModule, RubyModule[]> NO_REFINEMENTS = Collections.emptyMap();
3535

3636
/** @see <a href="http://yugui.jp/articles/846">http://yugui.jp/articles/846</a> */
3737
private interface DefaultDefinee {

0 commit comments

Comments
 (0)