Skip to content

Commit 32a0b86

Browse files
committed
Address comments per review
* Defer initialization of shared member node, add @child annotations * Reduce footprint of truffle boundaries * Revert bugfix when reflecting upon modules with globbed imports
1 parent f40ef4e commit 32a0b86

File tree

12 files changed

+65
-95
lines changed

12 files changed

+65
-95
lines changed

pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2224,8 +2224,7 @@ public Object visitReadExpr(ReadExprContext ctx) {
22242224
return ReadOrNullNodeGen.create(createSourceSection(ctx), moduleKey, visitExpr(exprCtx));
22252225
}
22262226
assert tokenType == PklLexer.READ_GLOB;
2227-
return ReadGlobNodeGen.create(
2228-
language, createSourceSection(ctx), moduleKey, visitExpr(exprCtx));
2227+
return ReadGlobNodeGen.create(createSourceSection(ctx), moduleKey, visitExpr(exprCtx));
22292228
}
22302229

22312230
@Override
@@ -2666,8 +2665,7 @@ private AbstractImportNode doVisitImport(
26662665
}
26672666
var resolvedUri = resolveImport(importUri, importUriCtx);
26682667
if (isGlobImport) {
2669-
return new ImportGlobNode(
2670-
language, section, moduleInfo.getResolvedModuleKey(), resolvedUri, importUri);
2668+
return new ImportGlobNode(section, moduleInfo.getResolvedModuleKey(), resolvedUri, importUri);
26712669
}
26722670
return new ImportNode(language, section, moduleInfo.getResolvedModuleKey(), resolvedUri);
26732671
}

pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobElementNode.java renamed to pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobMemberBodyNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@
3232
import org.pkl.core.util.GlobResolver.ResolvedGlobElement;
3333

3434
/** Used by {@link ReadGlobNode}. */
35-
public final class ImportGlobElementNode extends ExpressionNode {
35+
public final class ImportGlobMemberBodyNode extends ExpressionNode {
3636
private final VmLanguage language;
3737
private final ResolvedModuleKey currentModule;
3838

39-
public ImportGlobElementNode(
39+
public ImportGlobMemberBodyNode(
4040
SourceSection sourceSection, VmLanguage language, ResolvedModuleKey currentModule) {
4141
super(sourceSection);
4242
this.language = language;

pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ImportGlobNode.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,26 +39,32 @@
3939
@NodeInfo(shortName = "import*")
4040
public class ImportGlobNode extends AbstractImportNode {
4141
private final String globPattern;
42-
private final SharedMemberNode importGlobElementNode;
43-
42+
@Child @LateInit private SharedMemberNode memberNode;
4443
@CompilationFinal @LateInit private VmMapping cachedResult;
4544

4645
public ImportGlobNode(
47-
VmLanguage language,
4846
SourceSection sourceSection,
4947
ResolvedModuleKey currentModule,
5048
URI importUri,
5149
String globPattern) {
5250
super(sourceSection, currentModule, importUri);
5351
this.globPattern = globPattern;
54-
importGlobElementNode =
55-
new SharedMemberNode(
56-
sourceSection,
57-
sourceSection,
58-
"",
59-
language,
60-
new FrameDescriptor(),
61-
new ImportGlobElementNode(sourceSection, language, currentModule));
52+
}
53+
54+
private SharedMemberNode getMemberNode() {
55+
if (memberNode == null) {
56+
CompilerDirectives.transferToInterpreterAndInvalidate();
57+
var language = VmLanguage.get(this);
58+
memberNode =
59+
new SharedMemberNode(
60+
sourceSection,
61+
sourceSection,
62+
"",
63+
language,
64+
new FrameDescriptor(),
65+
new ImportGlobMemberBodyNode(sourceSection, language, currentModule));
66+
}
67+
return memberNode;
6268
}
6369

6470
@Override
@@ -81,9 +87,9 @@ public Object executeGeneric(VirtualFrame frame) {
8187
currentModule.getOriginal(),
8288
currentModule.getUri(),
8389
globPattern);
84-
var builder = new VmObjectBuilder();
90+
var builder = new VmObjectBuilder(resolvedElements.size());
8591
for (var entry : resolvedElements.entrySet()) {
86-
builder.addEntry(entry.getKey(), importGlobElementNode);
92+
builder.addEntry(entry.getKey(), getMemberNode());
8793
}
8894
cachedResult = builder.toMapping(resolvedElements);
8995
return cachedResult;

pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobElementNode.java renamed to pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobMemberBodyNode.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.pkl.core.ast.expression.unary;
1717

18-
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
18+
import com.oracle.truffle.api.CompilerDirectives;
1919
import com.oracle.truffle.api.frame.VirtualFrame;
2020
import com.oracle.truffle.api.source.SourceSection;
2121
import java.util.Map;
@@ -26,8 +26,8 @@
2626
import org.pkl.core.util.GlobResolver.ResolvedGlobElement;
2727

2828
/** Used by {@link ReadGlobNode}. */
29-
public class ReadGlobElementNode extends ExpressionNode {
30-
public ReadGlobElementNode(SourceSection sourceSection) {
29+
public class ReadGlobMemberBodyNode extends ExpressionNode {
30+
public ReadGlobMemberBodyNode(SourceSection sourceSection) {
3131
super(sourceSection);
3232
}
3333

@@ -38,13 +38,13 @@ public Object executeGeneric(VirtualFrame frame) {
3838
return readResource(mapping, path);
3939
}
4040

41-
@TruffleBoundary
4241
private Object readResource(VmObjectLike mapping, String path) {
4342
@SuppressWarnings("unchecked")
4443
var globElements = (Map<String, ResolvedGlobElement>) mapping.getExtraStorage();
45-
var resourceUri = globElements.get(path).getUri();
44+
var resourceUri = VmUtils.getMapValue(globElements, path).getUri();
4645
var resource = VmContext.get(this).getResourceManager().read(resourceUri, this).orElse(null);
4746
if (resource == null) {
47+
CompilerDirectives.transferToInterpreter();
4848
throw exceptionBuilder().evalError("cannotFindResource", resourceUri).build();
4949
}
5050
return resource;

pkl-core/src/main/java/org/pkl/core/ast/expression/unary/ReadGlobNode.java

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.pkl.core.ast.expression.unary;
1717

18+
import com.oracle.truffle.api.CompilerDirectives;
1819
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
1920
import com.oracle.truffle.api.dsl.Specialization;
2021
import com.oracle.truffle.api.frame.FrameDescriptor;
@@ -32,23 +33,31 @@
3233
import org.pkl.core.runtime.VmObjectBuilder;
3334
import org.pkl.core.util.GlobResolver;
3435
import org.pkl.core.util.GlobResolver.InvalidGlobPatternException;
36+
import org.pkl.core.util.LateInit;
3537

3638
@NodeInfo(shortName = "read*")
3739
public abstract class ReadGlobNode extends AbstractReadNode {
38-
private final SharedMemberNode readGlobElementNode;
3940
private final EconomicMap<String, VmMapping> cachedResults = EconomicMap.create();
41+
@Child @LateInit private SharedMemberNode memberNode;
4042

41-
protected ReadGlobNode(
42-
VmLanguage language, SourceSection sourceSection, ModuleKey currentModule) {
43+
protected ReadGlobNode(SourceSection sourceSection, ModuleKey currentModule) {
4344
super(sourceSection, currentModule);
44-
readGlobElementNode =
45-
new SharedMemberNode(
46-
sourceSection,
47-
sourceSection,
48-
"",
49-
language,
50-
new FrameDescriptor(),
51-
new ReadGlobElementNode(sourceSection));
45+
}
46+
47+
private SharedMemberNode getMemberNode() {
48+
if (memberNode == null) {
49+
CompilerDirectives.transferToInterpreterAndInvalidate();
50+
var language = VmLanguage.get(this);
51+
memberNode =
52+
new SharedMemberNode(
53+
sourceSection,
54+
sourceSection,
55+
"",
56+
language,
57+
new FrameDescriptor(),
58+
new ReadGlobMemberBodyNode(sourceSection));
59+
}
60+
return memberNode;
5261
}
5362

5463
@Specialization
@@ -76,9 +85,9 @@ public Object read(String globPattern) {
7685
currentModule,
7786
currentModule.getUri(),
7887
globPattern);
79-
var builder = new VmObjectBuilder();
88+
var builder = new VmObjectBuilder(resolvedElements.size());
8089
for (var entry : resolvedElements.entrySet()) {
81-
builder.addEntry(entry.getKey(), readGlobElementNode);
90+
builder.addEntry(entry.getKey(), getMemberNode());
8291
}
8392
cachedResult = builder.toMapping(resolvedElements);
8493
cachedResults.put(globPattern, cachedResult);

pkl-core/src/main/java/org/pkl/core/runtime/VmMap.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -225,23 +225,19 @@ public void force(boolean allowUndefinedValues) {
225225
}
226226
}
227227

228-
@TruffleBoundary
229228
public VmMapping toMapping() {
230-
var builder = new VmObjectBuilder(keyOrder.size());
231-
for (var key : keyOrder) {
232-
var value = map.get(key);
233-
assert value != null;
234-
builder.addEntry(key, value);
229+
var builder = new VmObjectBuilder(getLength());
230+
for (var entry : this) {
231+
builder.addEntry(VmUtils.getKey(entry), VmUtils.getValue(entry));
235232
}
236233
return builder.toMapping();
237234
}
238235

239-
@TruffleBoundary
240236
public VmDynamic toDynamic() {
241-
var builder = new VmObjectBuilder(keyOrder.size());
242-
for (var key : keyOrder) {
243-
var value = map.get(key);
244-
assert value != null;
237+
var builder = new VmObjectBuilder(getLength());
238+
for (var entry : this) {
239+
var key = VmUtils.getKey(entry);
240+
var value = VmUtils.getValue(entry);
245241
if (key instanceof String) {
246242
builder.addProperty(Identifier.get((String) key), value);
247243
} else {

pkl-core/src/main/java/org/pkl/core/runtime/VmObjectBuilder.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ public final class VmObjectBuilder {
2626
private final EconomicMap<Object, ObjectMember> members;
2727
private int elementCount = 0;
2828

29-
public VmObjectBuilder() {
30-
members = EconomicMaps.create();
31-
}
32-
3329
public VmObjectBuilder(int initialSize) {
3430
members = EconomicMaps.create(initialSize);
3531
}

pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.pkl.core.Composite;
2424
import org.pkl.core.PModule;
2525
import org.pkl.core.PObject;
26-
import org.pkl.core.ast.expression.unary.AbstractImportNode;
26+
import org.pkl.core.ast.expression.unary.ImportNode;
2727
import org.pkl.core.ast.member.ObjectMember;
2828
import org.pkl.core.util.EconomicMaps;
2929
import org.pkl.core.util.LateInit;
@@ -105,7 +105,7 @@ public VmMap getImports() {
105105
assert memberNode != null; // import is never a constant
106106
builder.add(
107107
member.getName().toString(),
108-
((AbstractImportNode) memberNode.getBodyNode()).getImportUri().toString());
108+
((ImportNode) memberNode.getBodyNode()).getImportUri().toString());
109109
}
110110
}
111111
return builder.build();

pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,4 +844,9 @@ public static int findSlot(VirtualFrame frame, Object identifier) {
844844
public static int findAuxiliarySlot(VirtualFrame frame, Object identifier) {
845845
return frame.getFrameDescriptor().getAuxiliarySlots().getOrDefault(identifier, -1);
846846
}
847+
848+
@TruffleBoundary
849+
public static <K, V> V getMapValue(Map<K, V> map, K key) {
850+
return map.get(key);
851+
}
847852
}

pkl-core/src/main/java/org/pkl/core/stdlib/base/MapNodes.java

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,11 @@
2121
import com.oracle.truffle.api.nodes.LoopNode;
2222
import java.util.Map;
2323
import org.pkl.core.ast.lambda.*;
24-
import org.pkl.core.ast.member.ObjectMember;
2524
import org.pkl.core.runtime.*;
2625
import org.pkl.core.stdlib.ExternalMethod0Node;
2726
import org.pkl.core.stdlib.ExternalMethod1Node;
2827
import org.pkl.core.stdlib.ExternalMethod2Node;
2928
import org.pkl.core.stdlib.ExternalPropertyNode;
30-
import org.pkl.core.util.EconomicMaps;
3129
import org.pkl.core.util.MutableReference;
3230

3331
public final class MapNodes {
@@ -238,28 +236,7 @@ protected VmMap eval(VmMap self) {
238236
public abstract static class toDynamic extends ExternalMethod0Node {
239237
@Specialization
240238
protected VmDynamic eval(VmMap self) {
241-
var members = EconomicMaps.<Object, ObjectMember>create(self.getLength());
242-
243-
for (var entry : self) {
244-
var key = VmUtils.getKey(entry);
245-
246-
if (key instanceof String string) {
247-
var name = Identifier.get(string);
248-
EconomicMaps.put(
249-
members,
250-
name,
251-
VmUtils.createSyntheticObjectProperty(name, "", VmUtils.getValue(entry)));
252-
} else {
253-
EconomicMaps.put(
254-
members, key, VmUtils.createSyntheticObjectEntry("", VmUtils.getValue(entry)));
255-
}
256-
}
257-
258-
return new VmDynamic(
259-
VmUtils.createEmptyMaterializedFrame(),
260-
BaseModule.getDynamicClass().getPrototype(),
261-
members,
262-
0);
239+
return self.toDynamic();
263240
}
264241
}
265242

0 commit comments

Comments
 (0)