Skip to content

Commit 33d2de5

Browse files
timfeltimfel
committed
[GR-17161] Improve performance of Set operations to benefit meteor3
PullRequest: graalpython/594
2 parents e831db9 + e8f0ff1 commit 33d2de5

File tree

5 files changed

+94
-23
lines changed

5 files changed

+94
-23
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/HashingStorageNodes.java

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,13 @@
127127
import com.oracle.truffle.api.object.Location;
128128
import com.oracle.truffle.api.object.Property;
129129
import com.oracle.truffle.api.object.Shape;
130+
import com.oracle.truffle.api.profiles.BranchProfile;
130131
import com.oracle.truffle.api.profiles.ConditionProfile;
131132
import com.oracle.truffle.api.profiles.ValueProfile;
132133

133134
@GenerateNodeFactory
134135
public abstract class HashingStorageNodes {
136+
private static final int MAX_STORAGES = 8;
135137

136138
public static class PythonEquivalence extends Equivalence {
137139
@Child private PRaiseNode raise;
@@ -1650,10 +1652,29 @@ public static ExclusiveOrNode create() {
16501652
}
16511653

16521654
public abstract static class KeysIsSubsetNode extends DictStorageBaseNode {
1655+
protected static final int MAX_STORAGES = HashingStorageNodes.MAX_STORAGES;
16531656

16541657
public abstract boolean execute(VirtualFrame frame, HashingStorage left, HashingStorage right);
16551658

1656-
@Specialization
1659+
@Specialization(limit = "MAX_STORAGES", guards = {"left.getClass() == leftClass", "right.getClass() == rightClass"})
1660+
public boolean isSubsetCached(VirtualFrame frame, HashingStorage left, HashingStorage right,
1661+
@Cached("left.getClass()") Class<? extends HashingStorage> leftClass,
1662+
@Cached("right.getClass()") Class<? extends HashingStorage> rightClass,
1663+
@Cached("create()") ContainsKeyNode containsKeyNode,
1664+
@Cached("createBinaryProfile()") ConditionProfile sizeProfile) {
1665+
if (sizeProfile.profile(leftClass.cast(left).length() > rightClass.cast(right).length())) {
1666+
return false;
1667+
}
1668+
1669+
for (Object leftKey : leftClass.cast(left).keys()) {
1670+
if (!containsKeyNode.execute(frame, right, leftKey)) {
1671+
return false;
1672+
}
1673+
}
1674+
return true;
1675+
}
1676+
1677+
@Specialization(replaces = "isSubsetCached")
16571678
public boolean isSubset(VirtualFrame frame, HashingStorage left, HashingStorage right,
16581679
@Cached("create()") ContainsKeyNode containsKeyNode,
16591680
@Cached("createBinaryProfile()") ConditionProfile sizeProfile) {
@@ -1692,24 +1713,61 @@ public static KeysIsSupersetNode create() {
16921713
}
16931714

16941715
public abstract static class DiffNode extends DictStorageBaseNode {
1716+
protected static final int MAX_STORAGES = HashingStorageNodes.MAX_STORAGES;
16951717

16961718
public abstract HashingStorage execute(VirtualFrame frame, HashingStorage left, HashingStorage right);
16971719

1698-
@Specialization(guards = "left.length() == 0")
1720+
protected boolean isEmpty(Class<? extends HashingStorage> theClass, HashingStorage s) {
1721+
return theClass.cast(s).length() == 0;
1722+
}
1723+
1724+
@Specialization(limit = "MAX_STORAGES", guards = {"left.getClass() == leftClass", "isEmpty(leftClass, left)"})
16991725
@SuppressWarnings("unused")
1700-
public HashingStorage doLeftEmpty(HashingStorage left, HashingStorage right) {
1726+
public HashingStorage doLeftEmpty(HashingStorage left, HashingStorage right,
1727+
@SuppressWarnings("unused") @Cached("left.getClass()") Class<? extends HashingStorage> leftClass) {
17011728
return EconomicMapStorage.create(false);
17021729
}
17031730

1704-
@Specialization(guards = "right.length() == 0")
1731+
@Specialization(limit = "MAX_STORAGES", guards = {"left.getClass() == leftClass", "right.getClass() == rightClass"})
1732+
@SuppressWarnings("try")
1733+
public HashingStorage doNonEmptyCached(VirtualFrame frame, HashingStorage left, HashingStorage right,
1734+
@Cached("left.getClass()") Class<? extends HashingStorage> leftClass,
1735+
@Cached("right.getClass()") Class<? extends HashingStorage> rightClass,
1736+
@Cached("create()") ContainsKeyNode containsKeyNode,
1737+
@Cached BranchProfile leftEmpty,
1738+
@Cached BranchProfile rightEmpty,
1739+
@Cached BranchProfile neitherEmpty,
1740+
@Cached("create()") SetItemNode setItemNode) {
1741+
if (leftClass.cast(left).length() == 0) {
1742+
leftEmpty.enter();
1743+
return EconomicMapStorage.create(false);
1744+
}
1745+
if (rightClass.cast(right).length() == 0) {
1746+
rightEmpty.enter();
1747+
try (DefaultContextManager cm = withGlobalState(frame)) {
1748+
return leftClass.cast(left).copy(getEquivalence());
1749+
}
1750+
}
1751+
neitherEmpty.enter();
1752+
HashingStorage newStorage = EconomicMapStorage.create(false);
1753+
for (Object leftKey : leftClass.cast(left).keys()) {
1754+
if (!containsKeyNode.execute(frame, right, leftKey)) {
1755+
newStorage = setItemNode.execute(frame, newStorage, leftKey, PNone.NO_VALUE);
1756+
}
1757+
}
1758+
return newStorage;
1759+
}
1760+
1761+
@Specialization(limit = "MAX_STORAGES", guards = {"right.getClass() == rightClass", "isEmpty(rightClass, right)"})
17051762
@SuppressWarnings("try")
1706-
public HashingStorage doRightEmpty(VirtualFrame frame, HashingStorage left, @SuppressWarnings("unused") HashingStorage right) {
1763+
public HashingStorage doRightEmpty(VirtualFrame frame, HashingStorage left, @SuppressWarnings("unused") HashingStorage right,
1764+
@SuppressWarnings("unused") @Cached("right.getClass()") Class<? extends HashingStorage> rightClass) {
17071765
try (DefaultContextManager cm = withGlobalState(frame)) {
17081766
return left.copy(getEquivalence());
17091767
}
17101768
}
17111769

1712-
@Specialization(guards = {"left.length() != 0", "right.length() != 0"})
1770+
@Specialization(replaces = "doNonEmptyCached")
17131771
public HashingStorage doNonEmpty(VirtualFrame frame, HashingStorage left, HashingStorage right,
17141772
@Cached("create()") ContainsKeyNode containsKeyNode,
17151773
@Cached("create()") SetItemNode setItemNode) {
@@ -1731,7 +1789,7 @@ public static DiffNode create() {
17311789
@GenerateUncached
17321790
public abstract static class LenNode extends Node {
17331791

1734-
protected static final int MAX_STORAGES = 8;
1792+
protected static final int MAX_STORAGES = HashingStorageNodes.MAX_STORAGES;
17351793

17361794
public abstract int execute(HashingStorage s);
17371795

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -556,8 +556,9 @@ protected Object doScalarGeneric(SequenceStorage storage, Object idx) {
556556

557557
@Specialization
558558
protected Object doSlice(SequenceStorage storage, PSlice slice,
559+
@Cached LenNode lenNode,
559560
@Cached PythonObjectFactory factory) {
560-
SliceInfo info = slice.computeIndices(storage.length());
561+
SliceInfo info = slice.computeIndices(lenNode.execute(storage));
561562
if (factoryMethod != null) {
562563
return factoryMethod.apply(getGetItemSliceNode().execute(storage, info.start, info.stop, info.step, info.length), factory);
563564
}
@@ -3295,7 +3296,7 @@ public ToArrayNode(boolean exact) {
32953296
@Specialization
32963297
Object[] doObjectSequenceStorage(ObjectSequenceStorage s) {
32973298
Object[] barr = s.getInternalArray();
3298-
if (exact) {
3299+
if (exact && barr.length != s.length()) {
32993300
return exactCopy(barr, s.length());
33003301
}
33013302
return barr;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/iterator/IteratorBuiltins.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEXT__;
3131
import static com.oracle.graal.python.runtime.exception.PythonErrorType.StopIteration;
3232

33+
import java.util.Iterator;
3334
import java.util.List;
3435

3536
import com.oracle.graal.python.builtins.Builtin;
@@ -49,6 +50,7 @@
4950
import com.oracle.graal.python.runtime.exception.PException;
5051
import com.oracle.graal.python.runtime.sequence.PSequence;
5152
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
53+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5254
import com.oracle.truffle.api.dsl.Cached;
5355
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
5456
import com.oracle.truffle.api.dsl.NodeFactory;
@@ -134,12 +136,23 @@ public long next(PLongSequenceIterator self) {
134136

135137
@Specialization
136138
public Object next(PBaseSetIterator self) {
137-
if (self.hasNext()) {
138-
return self.next();
139+
Iterator<Object> iterator = self.getIterator();
140+
if (hasNext(iterator)) {
141+
return getNext(iterator);
139142
}
140143
throw raise(StopIteration);
141144
}
142145

146+
@TruffleBoundary
147+
private static Object getNext(Iterator<Object> iterator) {
148+
return iterator.next();
149+
}
150+
151+
@TruffleBoundary
152+
private static boolean hasNext(Iterator<Object> iterator) {
153+
return iterator.hasNext();
154+
}
155+
143156
@Specialization(guards = "self.isPList()")
144157
public Object nextList(PSequenceIterator self,
145158
@Cached("createClassProfile()") ValueProfile storageProfile) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/positional/ExecutePositionalStarargsNode.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Iterator;
4545

4646
import com.oracle.graal.python.builtins.objects.PNone;
47+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
4748
import com.oracle.graal.python.builtins.objects.dict.PDict;
4849
import com.oracle.graal.python.builtins.objects.list.PList;
4950
import com.oracle.graal.python.builtins.objects.set.PSet;
@@ -81,13 +82,9 @@ static Object[] starargs(PTuple starargs) {
8182
}
8283

8384
@Specialization
84-
static Object[] starargs(PList starargs) {
85-
int length = starargs.getSequenceStorage().length();
86-
Object[] internalArray = starargs.getSequenceStorage().getInternalArray();
87-
if (internalArray.length != length) {
88-
return starargs.getSequenceStorage().getCopyOfInternalArray();
89-
}
90-
return internalArray;
85+
static Object[] starargs(PList starargs,
86+
@Cached SequenceStorageNodes.ToArrayNode toArray) {
87+
return toArray.execute(starargs.getSequenceStorage());
9188
}
9289

9390
@Specialization
@@ -170,7 +167,12 @@ static Object[] starargs(PTuple starargs) {
170167

171168
@Specialization
172169
static Object[] starargs(PList starargs) {
173-
return ExecutePositionalStarargsNode.starargs(starargs);
170+
int length = starargs.getSequenceStorage().length();
171+
Object[] internalArray = starargs.getSequenceStorage().getInternalArray();
172+
if (internalArray.length != length) {
173+
return starargs.getSequenceStorage().getCopyOfInternalArray();
174+
}
175+
return internalArray;
174176
}
175177

176178
@Specialization

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/BasicSequenceStorage.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2018, Oracle and/or its affiliates.
2+
* Copyright (c) 2017, 2019, Oracle and/or its affiliates.
33
* Copyright (c) 2013, Regents of the University of California
44
*
55
* All rights reserved.
@@ -25,8 +25,6 @@
2525
*/
2626
package com.oracle.graal.python.runtime.sequence.storage;
2727

28-
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
29-
3028
public abstract class BasicSequenceStorage extends SequenceStorage {
3129

3230
// nominated storage length
@@ -52,7 +50,6 @@ public void setNewLength(int length) {
5250
/**
5351
* The capacity we should allocate for a given length.
5452
*/
55-
@TruffleBoundary(transferToInterpreterOnException = false, allowInlining = true)
5653
private static int capacityFor(int length) throws ArithmeticException {
5754
return Math.max(16, Math.multiplyExact(length, 2));
5855
}

0 commit comments

Comments
 (0)