Skip to content

Commit ce6d5e3

Browse files
committed
SortNodes: always try strings fast-path
Importing the threading module in ContextVar constructor used to activate the string only specialization SortSequenceStorageNode. When the threading import was removed, the generic object storage specialization, which does not guard against string only case, got activated first and the string only specialization would never get activated. The generic object storage specialization seems to perform poorly on SVM compared to the strings only one.
1 parent 80a1060 commit ce6d5e3

File tree

1 file changed

+22
-9
lines changed
  • graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common

1 file changed

+22
-9
lines changed

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

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -76,6 +76,9 @@
7676
import com.oracle.truffle.api.dsl.Specialization;
7777
import com.oracle.truffle.api.frame.VirtualFrame;
7878
import com.oracle.truffle.api.nodes.ExplodeLoop;
79+
import com.oracle.truffle.api.nodes.LoopNode;
80+
import com.oracle.truffle.api.profiles.ConditionProfile;
81+
import com.oracle.truffle.api.profiles.LoopConditionProfile;
7982
import com.oracle.truffle.api.profiles.ValueProfile;
8083

8184
public abstract class SortNodes {
@@ -214,9 +217,8 @@ void sort(DoubleSequenceStorage storage, @SuppressWarnings("unused") PNone keyfu
214217
}
215218
}
216219

217-
@Specialization(guards = "isStringOnly(storage)")
218220
@TruffleBoundary
219-
void sort(ObjectSequenceStorage storage, @SuppressWarnings("unused") PNone keyfunc, boolean reverse) {
221+
private static void sortStrings(ObjectSequenceStorage storage, boolean reverse) {
220222
Object[] array = storage.getInternalArray();
221223
int len = storage.length();
222224
Comparator<Object> comparator;
@@ -228,23 +230,34 @@ void sort(ObjectSequenceStorage storage, @SuppressWarnings("unused") PNone keyfu
228230
Arrays.sort(array, 0, len, comparator);
229231
}
230232

231-
@TruffleBoundary
232-
protected static boolean isStringOnly(ObjectSequenceStorage storage) {
233+
protected boolean isStringOnly(ObjectSequenceStorage storage, LoopConditionProfile isStringOnlyLoopProfile, ConditionProfile isStringOnlyBreakProfile) {
233234
int length = storage.length();
234235
Object[] array = storage.getInternalArray();
235-
for (int i = 0; i < length; i++) {
236+
for (int i = 0; isStringOnlyLoopProfile.profile(i < length); i++) {
236237
Object value = array[i];
237-
if (!(value instanceof String)) {
238+
if (isStringOnlyBreakProfile.profile(!(value instanceof String))) {
239+
LoopNode.reportLoopCount(this, i);
238240
return false;
239241
}
240242
}
243+
LoopNode.reportLoopCount(this, length);
241244
return true;
242245
}
243246

244247
@Specialization
245-
void sort(VirtualFrame frame, ObjectSequenceStorage storage, @SuppressWarnings("unused") PNone keyfunc, boolean reverse,
248+
void sortObjSeqStorage(VirtualFrame frame, ObjectSequenceStorage storage, @SuppressWarnings("unused") PNone keyfunc, boolean reverse,
249+
@Cached ConditionProfile isStringOnlyProfile,
250+
@Cached LoopConditionProfile isStringOnlyLoopProfile,
251+
@Cached("createCountingProfile()") ConditionProfile isStringOnlyBreakProfile,
246252
@Cached CallContext callContext) {
247-
sortWithoutKey(frame, storage.getInternalArray(), storage.length(), reverse, callContext);
253+
if (isStringOnlyProfile.profile(isStringOnly(storage, isStringOnlyLoopProfile, isStringOnlyBreakProfile))) {
254+
// Sorting of strings seems to be so much faster (especially on SVM) that it is
255+
// worth always checking for string only sequences and not replacing the strings
256+
// specialized code with generic object storage code
257+
sortStrings(storage, reverse);
258+
} else {
259+
sortWithoutKey(frame, storage.getInternalArray(), storage.length(), reverse, callContext);
260+
}
248261
}
249262

250263
@Specialization(guards = "!isPNone(keyfunc)")

0 commit comments

Comments
 (0)