Skip to content

Commit 3baccca

Browse files
woesswoess
committed
[GR-71784] Specialize for-of on array iterators.
PullRequest: js/3639
2 parents 9e577d5 + 9f3b61d commit 3baccca

File tree

7 files changed

+217
-54
lines changed

7 files changed

+217
-54
lines changed

‎graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2195,7 +2195,7 @@ private JavaScriptNode desugarForInOrOfBody(ForNode forNode, JavaScriptNode iter
21952195
wrappedBody = factory.createIteratorCloseWrapper(context, wrappedBody, iteratorVar.createReadNode(), false);
21962196
}
21972197

2198-
RepeatingNode repeatingNode = factory.createForOfRepeatingNode(iteratorNext, wrappedBody,
2198+
RepeatingNode repeatingNode = factory.createForOfRepeatingNode(iteratorVar.createReadNode(), iteratorNext, wrappedBody,
21992199
(JSWriteFrameSlotNode) nextResultVar.createWriteNode(null));
22002200
LoopNode loopNode = factory.createLoopNode(repeatingNode);
22012201
JavaScriptNode whileNode = forNode.isForOf()

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ArrayIteratorPrototypeBuiltins.java‎

Lines changed: 105 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,20 @@
4242

4343
import com.oracle.truffle.api.dsl.Bind;
4444
import com.oracle.truffle.api.dsl.Cached;
45-
import com.oracle.truffle.api.dsl.Cached.Shared;
4645
import com.oracle.truffle.api.dsl.Fallback;
4746
import com.oracle.truffle.api.dsl.GenerateCached;
4847
import com.oracle.truffle.api.dsl.GenerateInline;
48+
import com.oracle.truffle.api.dsl.ImportStatic;
4949
import com.oracle.truffle.api.dsl.Specialization;
50+
import com.oracle.truffle.api.interop.InteropLibrary;
51+
import com.oracle.truffle.api.interop.UnsupportedMessageException;
52+
import com.oracle.truffle.api.library.CachedLibrary;
5053
import com.oracle.truffle.api.nodes.Node;
5154
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
55+
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
5256
import com.oracle.truffle.api.profiles.InlinedIntValueProfile;
5357
import com.oracle.truffle.js.builtins.ArrayIteratorPrototypeBuiltinsFactory.ArrayIteratorNextNodeGen;
58+
import com.oracle.truffle.js.nodes.JavaScriptBaseNode;
5459
import com.oracle.truffle.js.nodes.access.CreateIterResultObjectNode;
5560
import com.oracle.truffle.js.nodes.access.ReadElementNode;
5661
import com.oracle.truffle.js.nodes.array.JSGetLengthNode;
@@ -59,6 +64,7 @@
5964
import com.oracle.truffle.js.nodes.function.JSBuiltin;
6065
import com.oracle.truffle.js.nodes.function.JSBuiltinNode;
6166
import com.oracle.truffle.js.runtime.Errors;
67+
import com.oracle.truffle.js.runtime.JSConfig;
6268
import com.oracle.truffle.js.runtime.JSContext;
6369
import com.oracle.truffle.js.runtime.JSRuntime;
6470
import com.oracle.truffle.js.runtime.builtins.BuiltinEnum;
@@ -108,27 +114,39 @@ protected Object createNode(JSContext context, JSBuiltin builtin, boolean constr
108114

109115
public abstract static class ArrayIteratorNextNode extends JSBuiltinNode {
110116

117+
@Child private CreateIterResultObjectNode createIterResultObjectNode;
118+
111119
protected ArrayIteratorNextNode(JSContext context, JSBuiltin builtin) {
112120
super(context, builtin);
121+
this.createIterResultObjectNode = CreateIterResultObjectNode.create(context);
122+
}
123+
124+
protected JSObject createIteratorResultObject(Object value, boolean done) {
125+
return createIterResultObjectNode.execute(value, done);
113126
}
114127

115128
@Specialization(guards = {"!isUndefined(array)"})
116129
JSObject doArrayIterator(JSArrayIteratorObject iterator,
117130
@Bind("iterator.getIteratedObject()") Object array,
118-
@Shared @Cached("create(getContext())") CreateIterResultObjectNode createIterResultObjectNode,
119131
@Cached("create(getContext())") ReadElementNode readElementNode,
120132
@Cached ArrayIteratorGetLengthNode getLengthNode,
121133
@Cached(inline = true) LongToIntOrDoubleNode toJSIndex,
122-
@Cached InlinedIntValueProfile iterationKindProfile) {
134+
@Cached InlinedIntValueProfile iterationKindProfile,
135+
@Cached InlinedConditionProfile skipLengthCheckBranch) {
123136
long index = iterator.getNextIndex();
124-
int itemKind = iterationKindProfile.profile(this, iterator.getIterationKind());
125-
long length = getLengthNode.execute(this, array, getJSContext());
126-
127-
if (index >= length) {
128-
iterator.setIteratedObject(Undefined.instance);
129-
return createIterResultObjectNode.execute(Undefined.instance, true);
137+
assert index >= 0;
138+
if (skipLengthCheckBranch.profile(this, iterator.isSkipGetLength())) {
139+
// Already checked in the caller, no need to get the length again.
140+
iterator.setSkipGetLength(false);
141+
} else {
142+
long length = getLengthNode.execute(this, array, getContext());
143+
if (index >= length) {
144+
iterator.setIteratedObject(Undefined.instance);
145+
return createIteratorResultObject(Undefined.instance, true);
146+
}
130147
}
131148

149+
int itemKind = iterationKindProfile.profile(this, iterator.getIterationKind());
132150
iterator.setNextIndex(index + 1);
133151
Object indexNumber = null;
134152
if ((itemKind & JSRuntime.ITERATION_KIND_KEY) != 0) {
@@ -146,13 +164,12 @@ JSObject doArrayIterator(JSArrayIteratorObject iterator,
146164
result = JSArray.createConstantObjectArray(getContext(), getRealm(), new Object[]{indexNumber, elementValue});
147165
}
148166
}
149-
return createIterResultObjectNode.execute(result, false);
167+
return createIteratorResultObject(result, false);
150168
}
151169

152170
@Specialization(guards = {"isUndefined(iterator.getIteratedObject())"})
153-
static JSObject doArrayIteratorDetached(@SuppressWarnings("unused") JSArrayIteratorObject iterator,
154-
@Cached("create(getContext())") @Shared CreateIterResultObjectNode createIterResultObjectNode) {
155-
return createIterResultObjectNode.execute(Undefined.instance, true);
171+
final JSObject doArrayIteratorDetached(@SuppressWarnings("unused") JSArrayIteratorObject iterator) {
172+
return createIteratorResultObject(Undefined.instance, true);
156173
}
157174

158175
@SuppressWarnings("unused")
@@ -161,33 +178,87 @@ static JSObject doIncompatibleReceiver(Object iterator) {
161178
throw Errors.createTypeError("not an Array Iterator");
162179
}
163180
}
164-
}
165181

166-
@GenerateInline
167-
@GenerateCached(false)
168-
abstract class ArrayIteratorGetLengthNode extends Node {
182+
@GenerateInline
183+
@GenerateCached(false)
184+
public abstract static class ArrayIteratorGetLengthNode extends JavaScriptBaseNode {
169185

170-
abstract long execute(Node node, Object array, JSContext context);
186+
public abstract long execute(Node node, Object array, JSContext context);
171187

172-
@Specialization
173-
static long doArray(JSArrayObject array, @SuppressWarnings("unused") JSContext context) {
174-
return JSArray.arrayGetLength(array);
175-
}
188+
@Specialization
189+
static long doArray(JSArrayObject array, @SuppressWarnings("unused") JSContext context) {
190+
return JSArray.arrayGetLength(array);
191+
}
176192

177-
@Specialization
178-
static long doTypedArray(Node node, JSTypedArrayObject typedArray, JSContext context,
179-
@Cached TypedArrayLengthNode typedArrayLengthNode,
180-
@Cached InlinedBranchProfile errorBranch) {
181-
if (JSArrayBufferView.isOutOfBounds(typedArray, context)) {
182-
errorBranch.enter(node);
183-
throw Errors.createTypeError("Cannot perform Array Iterator.prototype.next on a detached ArrayBuffer");
193+
@Specialization
194+
static long doTypedArray(Node node, JSTypedArrayObject typedArray, JSContext context,
195+
@Cached TypedArrayLengthNode typedArrayLengthNode,
196+
@Cached InlinedBranchProfile errorBranch) {
197+
if (JSArrayBufferView.isOutOfBounds(typedArray, context)) {
198+
errorBranch.enter(node);
199+
throw Errors.createTypeError("Cannot perform Array Iterator.prototype.next on a detached ArrayBuffer");
200+
}
201+
return typedArrayLengthNode.execute(node, typedArray, context);
202+
}
203+
204+
@Fallback
205+
static long doArrayLike(Object array, @SuppressWarnings("unused") JSContext context,
206+
@Cached(value = "create(context)", inline = false) JSGetLengthNode getLengthNode) {
207+
return getLengthNode.executeLong(array);
184208
}
185-
return typedArrayLengthNode.execute(node, typedArray, context);
186209
}
187210

188-
@Fallback
189-
static long doArrayLike(Object array, @SuppressWarnings("unused") JSContext context,
190-
@Cached(value = "create(context)", inline = false) JSGetLengthNode getLengthNode) {
191-
return getLengthNode.executeLong(array);
211+
/**
212+
* Like {@link ArrayIteratorGetLengthNode}, but designed to avoid observable side effects. Only
213+
* handles JS arrays, typed arrays, and foreign arrays (assumes getArraySize to well-behaved).
214+
* Returns {@value JSRuntime#INVALID_INTEGER_INDEX} for other object types or if getting the
215+
* length would throw an error.
216+
*/
217+
@ImportStatic(JSConfig.class)
218+
@GenerateInline
219+
@GenerateCached(false)
220+
public abstract static class ArrayIteratorGetLengthSafeNode extends JavaScriptBaseNode {
221+
222+
public abstract long execute(Node node, Object array);
223+
224+
@Specialization
225+
static long doArray(JSArrayObject array) {
226+
return JSArray.arrayGetLength(array);
227+
}
228+
229+
@Specialization
230+
static long doTypedArray(Node node, JSTypedArrayObject typedArray,
231+
@Cached TypedArrayLengthNode typedArrayLengthNode,
232+
@Cached InlinedBranchProfile errorBranch) {
233+
JSContext context = JSContext.get(node);
234+
if (JSArrayBufferView.isOutOfBounds(typedArray, context)) {
235+
errorBranch.enter(node);
236+
// Must call into the next method to ensure correct stack trace for the error.
237+
return JSRuntime.INVALID_INTEGER_INDEX;
238+
}
239+
return typedArrayLengthNode.execute(node, typedArray, context);
240+
}
241+
242+
@Specialization(guards = "isUndefined(array)")
243+
static long doUndefined(@SuppressWarnings("unused") Object array) {
244+
// Iterator is already done and detached.
245+
return 0L;
246+
}
247+
248+
@Specialization(guards = {"interop.hasArrayElements(array)", "isForeignObject(array)"}, limit = "InteropLibraryLimit")
249+
static long doForeignArray(Object array,
250+
@CachedLibrary("array") InteropLibrary interop) {
251+
try {
252+
return interop.getArraySize(array);
253+
} catch (UnsupportedMessageException e) {
254+
return JSRuntime.INVALID_INTEGER_INDEX;
255+
}
256+
}
257+
258+
@Fallback
259+
static long doArrayLike(@SuppressWarnings("unused") Object array) {
260+
// Must call into the next method to ensure correct stack trace for length getters.
261+
return JSRuntime.INVALID_INTEGER_INDEX;
262+
}
192263
}
193264
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,8 @@ public JavaScriptNode createDesugaredForAwaitOf(LoopNode loopNode) {
537537
return WhileNode.createDesugaredForAwaitOf(loopNode);
538538
}
539539

540-
public RepeatingNode createForOfRepeatingNode(JavaScriptNode nextResultNode, JavaScriptNode body, JSWriteFrameSlotNode writeNextValueNode) {
541-
return WhileNode.createForOfRepeatingNode(nextResultNode, body, writeNextValueNode);
540+
public RepeatingNode createForOfRepeatingNode(JavaScriptNode iteratorNode, JavaScriptNode nextResultNode, JavaScriptNode body, JSWriteFrameSlotNode writeNextValueNode) {
541+
return WhileNode.createForOfRepeatingNode(iteratorNode, nextResultNode, body, writeNextValueNode);
542542
}
543543

544544
public RepeatingNode createForRepeatingNode(JavaScriptNode condition, JavaScriptNode body, JavaScriptNode modify, FrameDescriptor frameDescriptor, JavaScriptNode isFirstNode,

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/AbstractRepeatingNode.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,20 @@ private void checkThreadInterrupted() {
8383
}
8484

8585
@Override
86-
public Object execute(VirtualFrame frame) {
86+
public final Object execute(VirtualFrame frame) {
8787
return executeRepeating(frame);
8888
}
8989

90+
@Override
91+
public final boolean executeBoolean(VirtualFrame frame) {
92+
return executeRepeating(frame);
93+
}
94+
95+
@Override
96+
public final Object executeRepeatingWithValue(VirtualFrame frame) {
97+
return RepeatingNode.super.executeRepeatingWithValue(frame);
98+
}
99+
90100
protected boolean materializationNeeded() {
91101
// If we are using tagged nodes, this node is already materialized.
92102
return !JSNodeUtil.isTaggedNode(bodyNode);

0 commit comments

Comments
 (0)