Skip to content

Commit fcd0b60

Browse files
author
Nicolas Laurent
committed
[GR-31350] Ensure every loop in PE code uses both reportLoopCount and LoopConditionProfile
PullRequest: truffleruby/2651
2 parents cefed62 + b356592 commit fcd0b60

File tree

6 files changed

+195
-99
lines changed

6 files changed

+195
-99
lines changed

‎src/main/java/org/truffleruby/core/array/ArrayCopyCompatibleRangeNode.java

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

12+
import com.oracle.truffle.api.nodes.LoopNode;
1213
import com.oracle.truffle.api.profiles.LoopConditionProfile;
1314
import org.truffleruby.core.array.library.ArrayStoreLibrary;
1415
import org.truffleruby.language.RubyBaseNode;
@@ -64,9 +65,14 @@ protected void copy(RubyArray dst, RubyArray src, int dstStart, int srcStart, in
6465
if (share.profile(!stores.isPrimitive(srcStore) &&
6566
isDstShared.executeIsShared(dst) &&
6667
!isSrcShared.executeIsShared(src))) {
67-
loopProfile.profileCounted(length);
68-
for (int i = 0; loopProfile.inject(i < length); ++i) {
69-
writeBarrierNode.executeWriteBarrier(stores.read(srcStore, i));
68+
int i = 0;
69+
try {
70+
loopProfile.profileCounted(length);
71+
for (; loopProfile.inject(i < length); ++i) {
72+
writeBarrierNode.executeWriteBarrier(stores.read(srcStore, i));
73+
}
74+
} finally {
75+
LoopNode.reportLoopCount(this, i);
7076
}
7177
}
7278
}

‎src/main/java/org/truffleruby/core/array/ArrayNodes.java

Lines changed: 148 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,14 @@ protected RubyArray mulOther(RubyArray array, int count,
181181
}
182182
final Object store = array.store;
183183
final Object newStore = arrays.allocator(store).allocate(newSize);
184-
loopProfile.profileCounted(count);
185-
for (int n = 0; loopProfile.inject(n < count); n++) {
186-
arrays.copyContents(store, 0, newStore, n * size, size);
184+
int n = 0;
185+
try {
186+
loopProfile.profileCounted(count);
187+
for (; loopProfile.inject(n < count); n++) {
188+
arrays.copyContents(store, 0, newStore, n * size, size);
189+
}
190+
} finally {
191+
LoopNode.reportLoopCount(this, n);
187192
}
188193

189194
final RubyClass logicalClass = array.getLogicalClass();
@@ -485,13 +490,18 @@ protected Object compactObjectsNonMutable(RubyArray array,
485490

486491
int m = 0;
487492

488-
loopProfile.profileCounted(size);
489-
for (int n = 0; loopProfile.inject(n < size); n++) {
490-
Object v = stores.read(store, n);
491-
if (v != nil) {
492-
arrayBuilder.appendValue(state, m, v);
493-
m++;
493+
int n = 0;
494+
try {
495+
loopProfile.profileCounted(size);
496+
for (; loopProfile.inject(n < size); n++) {
497+
Object v = stores.read(store, n);
498+
if (v != nil) {
499+
arrayBuilder.appendValue(state, m, v);
500+
m++;
501+
}
494502
}
503+
} finally {
504+
LoopNode.reportLoopCount(this, n);
495505
}
496506

497507
return createArray(arrayBuilder.finish(state, m), m);
@@ -526,13 +536,18 @@ protected Object compactObjectsNonMutable(RubyArray array,
526536

527537
int m = 0;
528538

529-
loopProfile.profileCounted(size);
530-
for (int n = 0; loopProfile.inject(n < size); n++) {
531-
Object v = stores.read(oldStore, n);
532-
if (v != nil) {
533-
mutableStores.write(newStore, m, v);
534-
m++;
539+
int n = 0;
540+
try {
541+
loopProfile.profileCounted(size);
542+
for (; loopProfile.inject(n < size); n++) {
543+
Object v = stores.read(oldStore, n);
544+
if (v != nil) {
545+
mutableStores.write(newStore, m, v);
546+
m++;
547+
}
535548
}
549+
} finally {
550+
LoopNode.reportLoopCount(this, n);
536551
}
537552

538553
stores.clear(oldStore, m, size - m);
@@ -607,14 +622,19 @@ protected RubyArray concatManyGeneral(RubyArray array, Object first, Object[] re
607622
Object store = cowNode.execute(array, 0, size);
608623

609624
RubyArray result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(first));
610-
loopProfile.profileCounted(rest.length);
611-
for (int i = 0; loopProfile.inject(i < rest.length); i++) {
612-
Object arg = rest[i];
613-
if (selfArgProfile.profile(arg == array)) {
614-
result = appendManyNode.executeAppendMany(array, createArray(store, size));
615-
} else {
616-
result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(arg));
625+
int i = 0;
626+
try {
627+
loopProfile.profileCounted(rest.length);
628+
for (; loopProfile.inject(i < rest.length); i++) {
629+
Object arg = rest[i];
630+
if (selfArgProfile.profile(arg == array)) {
631+
result = appendManyNode.executeAppendMany(array, createArray(store, size));
632+
} else {
633+
result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(arg));
634+
}
617635
}
636+
} finally {
637+
LoopNode.reportLoopCount(this, i);
618638
}
619639
return result;
620640
}
@@ -676,20 +696,24 @@ private Object delete(VirtualFrame frame, RubyArray array, Object value, Object
676696

677697
int i = 0;
678698
int n = 0;
679-
loopProfile.profileCounted(size);
680-
while (loopProfile.inject(n < size)) {
681-
final Object stored = oldStores.read(oldStore, n);
682-
683-
if (sameOrEqualNode.executeSameOrEqual(stored, value)) {
684-
checkFrozen(array);
685-
found = stored;
686-
n++;
687-
} else {
688-
newStores.write(newStore, i, oldStores.read(oldStore, n));
699+
try {
700+
loopProfile.profileCounted(size);
701+
while (loopProfile.inject(n < size)) {
702+
final Object stored = oldStores.read(oldStore, n);
689703

690-
i++;
691-
n++;
704+
if (sameOrEqualNode.executeSameOrEqual(stored, value)) {
705+
checkFrozen(array);
706+
found = stored;
707+
n++;
708+
} else {
709+
newStores.write(newStore, i, oldStores.read(oldStore, n));
710+
711+
i++;
712+
n++;
713+
}
692714
}
715+
} finally {
716+
LoopNode.reportLoopCount(this, n);
693717
}
694718

695719
if (i != n) {
@@ -854,15 +878,19 @@ protected boolean equalSamePrimitiveType(RubyArray a, RubyArray b,
854878
final Object aStore = a.store;
855879
final Object bStore = b.store;
856880

857-
loopProfile.profileCounted(aSize);
858-
for (int i = 0; loopProfile.inject(i < aSize); i++) {
859-
if (!sameOrEqualNode
860-
.executeSameOrEqual(stores.read(aStore, i), stores.read(bStore, i))) {
861-
falseProfile.enter();
862-
return false;
881+
int i = 0;
882+
try {
883+
loopProfile.profileCounted(aSize);
884+
for (; loopProfile.inject(i < aSize); i++) {
885+
if (!sameOrEqualNode
886+
.executeSameOrEqual(stores.read(aStore, i), stores.read(bStore, i))) {
887+
falseProfile.enter();
888+
return false;
889+
}
863890
}
891+
} finally {
892+
LoopNode.reportLoopCount(this, i);
864893
}
865-
866894
trueProfile.enter();
867895
return true;
868896
}
@@ -922,12 +950,17 @@ protected boolean eqlSamePrimitiveType(RubyArray a, RubyArray b,
922950
final Object aStore = a.store;
923951
final Object bStore = b.store;
924952

925-
loopProfile.profileCounted(aSize);
926-
for (int i = 0; loopProfile.inject(i < aSize); i++) {
927-
if (!eqlNode.execute(stores.read(aStore, i), stores.read(bStore, i))) {
928-
falseProfile.enter();
929-
return false;
953+
int i = 0;
954+
try {
955+
loopProfile.profileCounted(aSize);
956+
for (; loopProfile.inject(i < aSize); i++) {
957+
if (!eqlNode.execute(stores.read(aStore, i), stores.read(bStore, i))) {
958+
falseProfile.enter();
959+
return false;
960+
}
930961
}
962+
} finally {
963+
LoopNode.reportLoopCount(this, i);
931964
}
932965

933966
trueProfile.enter();
@@ -974,9 +1007,14 @@ protected RubyArray fill(RubyArray array, Object[] args, Nil block,
9741007
final Object store = array.store;
9751008
final int size = array.size;
9761009

977-
loopProfile.profileCounted(size);
978-
for (int i = 0; loopProfile.inject(i < size); i++) {
979-
stores.write(store, i, value);
1010+
int i = 0;
1011+
try {
1012+
loopProfile.profileCounted(size);
1013+
for (; loopProfile.inject(i < size); i++) {
1014+
stores.write(store, i, value);
1015+
}
1016+
} finally {
1017+
LoopNode.reportLoopCount(this, i);
9801018
}
9811019
return array;
9821020
}
@@ -1016,11 +1054,16 @@ protected long hash(VirtualFrame frame, RubyArray array,
10161054
h = Hashing.update(h, CLASS_SALT);
10171055
final Object store = array.store;
10181056

1019-
loopProfile.profileCounted(size);
1020-
for (int n = 0; loopProfile.inject(n < size); n++) {
1021-
final Object value = stores.read(store, n);
1022-
final long valueHash = toLongNode.execute(toHashNode.call(value, "hash"));
1023-
h = Hashing.update(h, valueHash);
1057+
int n = 0;
1058+
try {
1059+
loopProfile.profileCounted(size);
1060+
for (; loopProfile.inject(n < size); n++) {
1061+
final Object value = stores.read(store, n);
1062+
final long valueHash = toLongNode.execute(toHashNode.call(value, "hash"));
1063+
h = Hashing.update(h, valueHash);
1064+
}
1065+
} finally {
1066+
LoopNode.reportLoopCount(this, n);
10241067
}
10251068

10261069
return Hashing.end(h);
@@ -1040,13 +1083,18 @@ protected boolean include(RubyArray array, Object value,
10401083
@Cached LoopConditionProfile loopProfile) {
10411084
final Object store = array.store;
10421085

1043-
loopProfile.profileCounted(array.size);
1044-
for (int n = 0; loopProfile.inject(n < array.size); n++) {
1045-
final Object stored = stores.read(store, n);
1086+
int n = 0;
1087+
try {
1088+
loopProfile.profileCounted(array.size);
1089+
for (; loopProfile.inject(n < array.size); n++) {
1090+
final Object stored = stores.read(store, n);
10461091

1047-
if (sameOrEqualNode.executeSameOrEqual(stored, value)) {
1048-
return true;
1092+
if (sameOrEqualNode.executeSameOrEqual(stored, value)) {
1093+
return true;
1094+
}
10491095
}
1096+
} finally {
1097+
LoopNode.reportLoopCount(this, n);
10501098
}
10511099

10521100
return false;
@@ -1126,9 +1174,14 @@ protected RubyArray initializeWithSizeAndValue(RubyArray array, int size, Object
11261174
final Object allocatedStore = stores.allocateForNewValue(array.store, fillingValue, size);
11271175
if (needsFill.profile(!allocatedStores.isDefaultValue(allocatedStore, fillingValue))) {
11281176
propagateSharingNode.executePropagate(array, fillingValue);
1129-
loopProfile.profileCounted(size);
1130-
for (int i = 0; loopProfile.inject(i < size); i++) {
1131-
allocatedStores.write(allocatedStore, i, fillingValue);
1177+
int i = 0;
1178+
try {
1179+
loopProfile.profileCounted(size);
1180+
for (; loopProfile.inject(i < size); i++) {
1181+
allocatedStores.write(allocatedStore, i, fillingValue);
1182+
}
1183+
} finally {
1184+
LoopNode.reportLoopCount(this, i);
11321185
}
11331186
}
11341187
setStoreAndSize(array, allocatedStore, size);
@@ -1656,13 +1709,17 @@ protected RubyArray pushMany(VirtualFrame frame, RubyArray array, Object value,
16561709
// NOTE (eregon): Appending one by one here to avoid useless generalization to Object[]
16571710
// if the arguments all fit in the current storage
16581711
appendOneNode.executeAppendOne(array, value);
1659-
loopProfile.profileCounted(rest.length);
1660-
for (int i = 0; loopProfile.inject(i < rest.length); i++) {
1661-
appendOneNode.executeAppendOne(array, rest[i]);
1712+
int i = 0;
1713+
try {
1714+
loopProfile.profileCounted(rest.length);
1715+
for (; loopProfile.inject(i < rest.length); i++) {
1716+
appendOneNode.executeAppendOne(array, rest[i]);
1717+
}
1718+
} finally {
1719+
LoopNode.reportLoopCount(this, i);
16621720
}
16631721
return array;
16641722
}
1665-
16661723
}
16671724

16681725
@CoreMethod(names = "reject", needsBlock = true, enumeratorSize = "size")
@@ -1934,16 +1991,19 @@ private void reverse(ArrayStoreLibrary stores,
19341991
Object store, int from, int until, LoopConditionProfile loopProfile) {
19351992
int to = until - 1;
19361993
final int loopCount = (until - from) >> 1;
1937-
loopProfile.profileCounted(loopCount);
1938-
while (loopProfile.inject(from < to)) {
1939-
final Object tmp = stores.read(store, from);
1940-
stores.write(store, from, stores.read(store, to));
1941-
stores.write(store, to, tmp);
1942-
from++;
1943-
to--;
1994+
try {
1995+
loopProfile.profileCounted(loopCount);
1996+
while (loopProfile.inject(from < to)) {
1997+
final Object tmp = stores.read(store, from);
1998+
stores.write(store, from, stores.read(store, to));
1999+
stores.write(store, to, tmp);
2000+
from++;
2001+
to--;
2002+
}
2003+
} finally {
2004+
LoopNode.reportLoopCount(this, loopCount);
19442005
}
19452006
}
1946-
19472007
}
19482008

19492009
@CoreMethod(names = { "select", "filter" }, needsBlock = true, enumeratorSize = "size")
@@ -2220,16 +2280,21 @@ protected RubyArray zipToPairs(RubyArray array, RubyArray other,
22202280

22212281
final Object[] zipped = new Object[zippedLength];
22222282

2223-
loopProfile.profileCounted(zippedLength);
2224-
for (int n = 0; loopProfile.inject(n < zippedLength); n++) {
2225-
if (bNotSmallerProfile.profile(n < bSize)) {
2226-
final Object pair = aStores.allocateForNewStore(a, b, 2);
2227-
pairs.write(pair, 0, aStores.read(a, n));
2228-
pairs.write(pair, 1, bStores.read(b, n));
2229-
zipped[n] = createArray(pair, 2);
2230-
} else {
2231-
zipped[n] = createArray(new Object[]{ aStores.read(a, n), nil });
2283+
int n = 0;
2284+
try {
2285+
loopProfile.profileCounted(zippedLength);
2286+
for (; loopProfile.inject(n < zippedLength); n++) {
2287+
if (bNotSmallerProfile.profile(n < bSize)) {
2288+
final Object pair = aStores.allocateForNewStore(a, b, 2);
2289+
pairs.write(pair, 0, aStores.read(a, n));
2290+
pairs.write(pair, 1, bStores.read(b, n));
2291+
zipped[n] = createArray(pair, 2);
2292+
} else {
2293+
zipped[n] = createArray(new Object[]{ aStores.read(a, n), nil });
2294+
}
22322295
}
2296+
} finally {
2297+
LoopNode.reportLoopCount(this, n);
22332298
}
22342299

22352300
return createArray(zipped, zippedLength);

0 commit comments

Comments
 (0)