Skip to content

Commit c384538

Browse files
eregoneregon
committed
[GR-19548] String performance fixes.
PullRequest: truffleruby/1143
2 parents 8aecf15 + 7091d80 commit c384538

File tree

5 files changed

+145
-120
lines changed

5 files changed

+145
-120
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ Performance:
4545
* `rb_type` information is now cached on classes as a hidden variable to improve performance.
4646
* Change to using thread local buffers for socket calls to reduce allocations.
4747
* Refactor `IO.select` to reduce copying and optimisation boundaries.
48+
* Refactor various `String` and `Rope` nodes to avoid Truffle performance warnings.
4849

4950
# 19.3.0
5051

‎src/main/java/org/truffleruby/core/rope/RopeGuards.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,12 @@ public static boolean isFixedWidthEncoding(Rope rope) {
4040
return rope.getEncoding().isFixedWidth();
4141
}
4242

43+
public static boolean is7Bit(Rope rope, RopeNodes.CodeRangeNode codeRangeNode) {
44+
return codeRangeNode.execute(rope) == CodeRange.CR_7BIT;
45+
}
46+
47+
public static boolean isAsciiCompatible(Rope rope) {
48+
return rope.getEncoding().isAsciiCompatible();
49+
}
50+
4351
}

‎src/main/java/org/truffleruby/core/rope/RopeNodes.java

Lines changed: 120 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@
2727
import org.jcodings.specific.ASCIIEncoding;
2828
import org.jcodings.specific.USASCIIEncoding;
2929
import org.jcodings.specific.UTF8Encoding;
30+
import org.truffleruby.core.array.ArrayUtils;
3031
import org.truffleruby.core.encoding.EncodingNodes;
32+
import org.truffleruby.core.rope.RopeNodesFactory.AreComparableRopesNodeGen;
33+
import org.truffleruby.core.rope.RopeNodesFactory.CompareRopesNodeGen;
3134
import org.truffleruby.core.rope.RopeNodesFactory.SetByteNodeGen;
3235
import org.truffleruby.core.string.StringAttributes;
3336
import org.truffleruby.core.string.StringSupport;
@@ -40,6 +43,7 @@
4043
import com.oracle.truffle.api.CompilerDirectives;
4144
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4245
import com.oracle.truffle.api.dsl.Cached;
46+
import com.oracle.truffle.api.dsl.Fallback;
4347
import com.oracle.truffle.api.dsl.GenerateUncached;
4448
import com.oracle.truffle.api.dsl.ImportStatic;
4549
import com.oracle.truffle.api.dsl.ReportPolymorphism;
@@ -606,7 +610,9 @@ private void linearizeTree(ManagedRope rope, Deque<ManagedRope> ropeQueue) {
606610

607611
@Specialization(guards = { "!left.isEmpty()", "!right.isEmpty()", "isCodeRangeBroken(left, right)" })
608612
protected Rope concatCrBroken(ManagedRope left, ManagedRope right, Encoding encoding,
609-
@Cached MakeLeafRopeNode makeLeafRopeNode) {
613+
@Cached MakeLeafRopeNode makeLeafRopeNode,
614+
@Cached BytesNode leftBytesNode,
615+
@Cached BytesNode rightBytesNode) {
610616
// This specialization was added to a special case where broken code range(s),
611617
// may concat to form a valid code range.
612618
try {
@@ -617,8 +623,8 @@ protected Rope concatCrBroken(ManagedRope left, ManagedRope right, Encoding enco
617623
getContext().getCoreExceptions().argumentErrorTooLargeString(this));
618624
}
619625

620-
final byte[] leftBytes = left.getBytes();
621-
final byte[] rightBytes = right.getBytes();
626+
final byte[] leftBytes = leftBytesNode.execute(left);
627+
final byte[] rightBytes = rightBytesNode.execute(right);
622628
final byte[] bytes = new byte[leftBytes.length + rightBytes.length];
623629
System.arraycopy(leftBytes, 0, bytes, 0, leftBytes.length);
624630
System.arraycopy(rightBytes, 0, bytes, leftBytes.length, rightBytes.length);
@@ -1798,4 +1804,115 @@ protected LeafRope nativeToManaged(NativeRope rope,
17981804

17991805
}
18001806

1807+
@ImportStatic(RopeGuards.class)
1808+
public abstract static class AreComparableRopesNode extends RubyBaseNode {
1809+
1810+
public static AreComparableRopesNode create() {
1811+
return AreComparableRopesNodeGen.create();
1812+
}
1813+
1814+
@Child CodeRangeNode codeRangeNode = RopeNodes.CodeRangeNode.create();
1815+
1816+
public abstract boolean execute(Rope firstRope, Rope secondRope);
1817+
1818+
@Specialization(guards = "a.getEncoding() == b.getEncoding()")
1819+
protected boolean sameEncoding(Rope a, Rope b) {
1820+
return true;
1821+
}
1822+
1823+
@Specialization(guards = "a.isEmpty()")
1824+
protected boolean firstEmpty(Rope a, Rope b) {
1825+
return true;
1826+
}
1827+
1828+
@Specialization(guards = "b.isEmpty()")
1829+
protected boolean secondEmpty(Rope a, Rope b) {
1830+
return true;
1831+
}
1832+
1833+
@Specialization(guards = { "is7Bit(a, codeRangeNode)", "is7Bit(b, codeRangeNode)" })
1834+
protected boolean bothCR7bit(Rope a, Rope b) {
1835+
return true;
1836+
}
1837+
1838+
@Specialization(guards = { "is7Bit(a, codeRangeNode)", "isAsciiCompatible(b)" })
1839+
protected boolean CR7bitASCII(Rope a, Rope b) {
1840+
return true;
1841+
}
1842+
1843+
@Specialization(guards = { "isAsciiCompatible(a)", "is7Bit(b, codeRangeNode)" })
1844+
protected boolean ASCIICR7bit(Rope a, Rope b) {
1845+
return true;
1846+
}
1847+
1848+
@Fallback
1849+
protected boolean notCompatible(Rope a, Rope b) {
1850+
return false;
1851+
}
1852+
1853+
}
1854+
1855+
public abstract static class CompareRopesNode extends RubyBaseNode {
1856+
1857+
public static CompareRopesNode create() {
1858+
return CompareRopesNodeGen.create();
1859+
}
1860+
1861+
public abstract int execute(Rope firstRope, Rope secondRope);
1862+
1863+
@Specialization
1864+
protected int compareRopes(Rope firstRope, Rope secondRope,
1865+
@Cached("createBinaryProfile()") ConditionProfile equalSubsequenceProfile,
1866+
@Cached("createBinaryProfile()") ConditionProfile equalLengthProfile,
1867+
@Cached("createBinaryProfile()") ConditionProfile firstStringShorterProfile,
1868+
@Cached("createBinaryProfile()") ConditionProfile greaterThanProfile,
1869+
@Cached("createBinaryProfile()") ConditionProfile equalProfile,
1870+
@Cached("createBinaryProfile()") ConditionProfile notComparableProfile,
1871+
@Cached("createBinaryProfile()") ConditionProfile encodingIndexGreaterThanProfile,
1872+
@Cached BytesNode firstBytesNode,
1873+
@Cached BytesNode secondBytesNode,
1874+
@Cached AreComparableRopesNode areComparableRopesNode) {
1875+
final boolean firstRopeShorter = firstStringShorterProfile
1876+
.profile(firstRope.byteLength() < secondRope.byteLength());
1877+
final int memcmpLength;
1878+
if (firstRopeShorter) {
1879+
memcmpLength = firstRope.byteLength();
1880+
} else {
1881+
memcmpLength = secondRope.byteLength();
1882+
}
1883+
1884+
final byte[] bytes = firstBytesNode.execute(firstRope);
1885+
final byte[] otherBytes = secondBytesNode.execute(secondRope);
1886+
1887+
final int ret;
1888+
final int cmp = ArrayUtils.memcmp(bytes, 0, otherBytes, 0, memcmpLength);
1889+
if (equalSubsequenceProfile.profile(cmp == 0)) {
1890+
if (equalLengthProfile.profile(firstRope.byteLength() == secondRope.byteLength())) {
1891+
ret = 0;
1892+
} else {
1893+
if (firstRopeShorter) {
1894+
ret = -1;
1895+
} else {
1896+
ret = 1;
1897+
}
1898+
}
1899+
} else {
1900+
ret = greaterThanProfile.profile(cmp > 0) ? 1 : -1;
1901+
}
1902+
1903+
if (equalProfile.profile(ret == 0)) {
1904+
if (notComparableProfile.profile(!areComparableRopesNode.execute(firstRope, secondRope))) {
1905+
if (encodingIndexGreaterThanProfile
1906+
.profile(firstRope.getEncoding().getIndex() > secondRope.getEncoding().getIndex())) {
1907+
return 1;
1908+
} else {
1909+
return -1;
1910+
}
1911+
}
1912+
}
1913+
1914+
return ret;
1915+
1916+
}
1917+
}
18011918
}

‎src/main/java/org/truffleruby/core/rope/RopeOperations.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -583,32 +583,6 @@ public static int hashForRange(Rope rope, int startingHashCode, int offset, int
583583
}
584584
}
585585

586-
public static boolean areComparable(Rope rope, Rope other) {
587-
// Taken from org.jruby.util.StringSupport.areComparable.
588-
589-
if (rope.getEncoding() == other.getEncoding() ||
590-
rope.isEmpty() || other.isEmpty()) {
591-
return true;
592-
}
593-
return areComparableViaCodeRange(rope, other);
594-
}
595-
596-
public static boolean areComparableViaCodeRange(Rope string, Rope other) {
597-
// Taken from org.jruby.util.StringSupport.areComparableViaCodeRange.
598-
599-
CodeRange cr1 = string.getCodeRange();
600-
CodeRange cr2 = other.getCodeRange();
601-
602-
if (cr1 == CR_7BIT && (cr2 == CR_7BIT || other.getEncoding().isAsciiCompatible())) {
603-
return true;
604-
}
605-
if (cr2 == CR_7BIT && string.getEncoding().isAsciiCompatible()) {
606-
return true;
607-
}
608-
return false;
609-
}
610-
611-
612586
public static RopeBuilder getRopeBuilderReadOnly(Rope rope) {
613587
return RopeBuilder.createRopeBuilder(rope.getBytes(), rope.getEncoding());
614588
}

‎src/main/java/org/truffleruby/core/string/StringNodes.java

Lines changed: 16 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
import org.truffleruby.core.rope.RopeGuards;
127127
import org.truffleruby.core.rope.RopeNodes;
128128
import org.truffleruby.core.rope.RopeNodes.RepeatNode;
129+
import org.truffleruby.core.rope.RopeNodes.SingleByteOptimizableNode;
129130
import org.truffleruby.core.rope.RopeOperations;
130131
import org.truffleruby.core.rope.SubstringRope;
131132
import org.truffleruby.core.string.StringNodesFactory.ByteIndexFromCharIndexNodeGen;
@@ -433,15 +434,7 @@ public abstract static class CompareNode extends CoreMethodArrayArgumentsNode {
433434
@Specialization(guards = "isRubyString(b)")
434435
protected int compare(DynamicObject a, DynamicObject b,
435436
@Cached("createBinaryProfile()") ConditionProfile sameRopeProfile,
436-
@Cached("createBinaryProfile()") ConditionProfile equalSubsequenceProfile,
437-
@Cached("createBinaryProfile()") ConditionProfile equalLengthProfile,
438-
@Cached("createBinaryProfile()") ConditionProfile firstStringShorterProfile,
439-
@Cached("createBinaryProfile()") ConditionProfile greaterThanProfile,
440-
@Cached("createBinaryProfile()") ConditionProfile equalProfile,
441-
@Cached("createBinaryProfile()") ConditionProfile notComparableProfile,
442-
@Cached("createBinaryProfile()") ConditionProfile encodingIndexGreaterThanProfile,
443-
@Cached RopeNodes.BytesNode firstBytesNode,
444-
@Cached RopeNodes.BytesNode secondBytesNode) {
437+
@Cached RopeNodes.CompareRopesNode compareNode) {
445438
// Taken from org.jruby.RubyString#op_cmp
446439

447440
final Rope firstRope = rope(a);
@@ -451,46 +444,7 @@ protected int compare(DynamicObject a, DynamicObject b,
451444
return 0;
452445
}
453446

454-
final boolean firstRopeShorter = firstStringShorterProfile
455-
.profile(firstRope.byteLength() < secondRope.byteLength());
456-
final int memcmpLength;
457-
if (firstRopeShorter) {
458-
memcmpLength = firstRope.byteLength();
459-
} else {
460-
memcmpLength = secondRope.byteLength();
461-
}
462-
463-
final byte[] bytes = firstBytesNode.execute(firstRope);
464-
final byte[] otherBytes = secondBytesNode.execute(secondRope);
465-
466-
final int ret;
467-
final int cmp = ArrayUtils.memcmp(bytes, 0, otherBytes, 0, memcmpLength);
468-
if (equalSubsequenceProfile.profile(cmp == 0)) {
469-
if (equalLengthProfile.profile(firstRope.byteLength() == secondRope.byteLength())) {
470-
ret = 0;
471-
} else {
472-
if (firstRopeShorter) {
473-
ret = -1;
474-
} else {
475-
ret = 1;
476-
}
477-
}
478-
} else {
479-
ret = greaterThanProfile.profile(cmp > 0) ? 1 : -1;
480-
}
481-
482-
if (equalProfile.profile(ret == 0)) {
483-
if (notComparableProfile.profile(!RopeOperations.areComparable(firstRope, secondRope))) {
484-
if (encodingIndexGreaterThanProfile
485-
.profile(firstRope.getEncoding().getIndex() > secondRope.getEncoding().getIndex())) {
486-
return 1;
487-
} else {
488-
return -1;
489-
}
490-
}
491-
}
492-
493-
return ret;
447+
return compareNode.execute(firstRope, secondRope);
494448
}
495449

496450
}
@@ -3442,49 +3396,14 @@ protected static boolean indexOutOfBounds(DynamicObject string, int byteIndex) {
34423396
@ImportStatic(StringGuards.class)
34433397
public static abstract class StringAreComparableNode extends RubyBaseNode {
34443398

3445-
@Child RopeNodes.CodeRangeNode codeRangeNode = RopeNodes.CodeRangeNode.create();
3399+
@Child RopeNodes.AreComparableRopesNode areComparableRopesNode = RopeNodes.AreComparableRopesNode.create();
34463400

34473401
public abstract boolean executeAreComparable(DynamicObject first, DynamicObject second);
34483402

3449-
@Specialization(guards = "getEncoding(a) == getEncoding(b)")
3403+
@Specialization
34503404
protected boolean sameEncoding(DynamicObject a, DynamicObject b) {
3451-
return true;
3452-
}
3453-
3454-
@Specialization(guards = "isEmpty(a)")
3455-
protected boolean firstEmpty(DynamicObject a, DynamicObject b) {
3456-
return true;
3405+
return areComparableRopesNode.execute(Layouts.STRING.getRope(a), Layouts.STRING.getRope(b));
34573406
}
3458-
3459-
@Specialization(guards = "isEmpty(b)")
3460-
protected boolean secondEmpty(DynamicObject a, DynamicObject b) {
3461-
return true;
3462-
}
3463-
3464-
@Specialization(guards = { "is7Bit(a, codeRangeNode)", "is7Bit(b, codeRangeNode)" })
3465-
protected boolean bothCR7bit(DynamicObject a, DynamicObject b) {
3466-
return true;
3467-
}
3468-
3469-
@Specialization(guards = { "is7Bit(a, codeRangeNode)", "isAsciiCompatible(b)" })
3470-
protected boolean CR7bitASCII(DynamicObject a, DynamicObject b) {
3471-
return true;
3472-
}
3473-
3474-
@Specialization(guards = { "isAsciiCompatible(a)", "is7Bit(b, codeRangeNode)" })
3475-
protected boolean ASCIICR7bit(DynamicObject a, DynamicObject b) {
3476-
return true;
3477-
}
3478-
3479-
@Fallback
3480-
protected boolean notCompatible(DynamicObject a, DynamicObject b) {
3481-
return false;
3482-
}
3483-
3484-
protected static Encoding getEncoding(DynamicObject string) {
3485-
return rope(string).getEncoding();
3486-
}
3487-
34883407
}
34893408

34903409
@ImportStatic({ StringGuards.class, StringOperations.class })
@@ -4260,26 +4179,28 @@ public static ByteIndexFromCharIndexNode create() {
42604179
return ByteIndexFromCharIndexNodeGen.create();
42614180
}
42624181

4182+
@Child protected SingleByteOptimizableNode singleByteOptimizableNode = SingleByteOptimizableNode.create();
4183+
42634184
public abstract int execute(Rope rope, int startByteOffset, int characterIndex);
42644185

4265-
@Specialization(guards = "rope.isSingleByteOptimizable()")
4186+
@Specialization(guards = "isSingleByteOptimizable(rope)")
42664187
protected int singleByteOptimizable(Rope rope, int startByteOffset, int characterIndex) {
42674188
return startByteOffset + characterIndex;
42684189
}
42694190

4270-
@Specialization(guards = { "!rope.isSingleByteOptimizable()", "isFixedWidthEncoding(rope)" })
4191+
@Specialization(guards = { "!isSingleByteOptimizable(rope)", "isFixedWidthEncoding(rope)" })
42714192
protected int fixedWidthEncoding(Rope rope, int startByteOffset, int characterIndex) {
42724193
final Encoding encoding = rope.getEncoding();
42734194
return startByteOffset + characterIndex * encoding.minLength();
42744195
}
42754196

42764197
@Specialization(
4277-
guards = { "!rope.isSingleByteOptimizable()", "!isFixedWidthEncoding(rope)", "characterIndex == 0" })
4198+
guards = { "!isSingleByteOptimizable(rope)", "!isFixedWidthEncoding(rope)", "characterIndex == 0" })
42784199
protected int multiByteZeroIndex(Rope rope, int startByteOffset, int characterIndex) {
42794200
return startByteOffset;
42804201
}
42814202

4282-
@Specialization(guards = { "!rope.isSingleByteOptimizable()", "!isFixedWidthEncoding(rope)" })
4203+
@Specialization(guards = { "!isSingleByteOptimizable(rope)", "!isFixedWidthEncoding(rope)" })
42834204
protected int multiBytes(Rope rope, int startByteOffset, int characterIndex,
42844205
@Cached("createBinaryProfile()") ConditionProfile indexTooLargeProfile,
42854206
@Cached("createBinaryProfile()") ConditionProfile invalidByteProfile,
@@ -4316,6 +4237,10 @@ protected int multiBytes(Rope rope, int startByteOffset, int characterIndex,
43164237
}
43174238
}
43184239

4240+
protected boolean isSingleByteOptimizable(Rope rope) {
4241+
return singleByteOptimizableNode.execute(rope);
4242+
}
4243+
43194244
}
43204245

43214246
// Named 'string_byte_index' in Rubinius.

0 commit comments

Comments
 (0)