Skip to content

Commit c1ea5db

Browse files
author
Nicolas Laurent
committed
replace ConcatChildren with ConcatState which also includes a bytes field
1 parent ae0ba4d commit c1ea5db

File tree

5 files changed

+81
-91
lines changed

5 files changed

+81
-91
lines changed

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

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,27 @@
1616

1717
public class ConcatRope extends ManagedRope {
1818

19+
/** Wrapper for the current state of the concat rope, including null children and a a byte array, or a null byte
20+
* array and the children. Accessing the state through {@link #getState()} guarantees the avoidance of race
21+
* conditions. */
1922
@ValueType
20-
public static class ConcatChildren {
23+
public static class ConcatState {
2124
public final ManagedRope left, right;
25+
public final byte[] bytes;
2226

23-
public ConcatChildren(ManagedRope left, ManagedRope right) {
27+
public ConcatState(ManagedRope left, ManagedRope right, byte[] bytes) {
28+
assert bytes == null && left != null && right != null || bytes != null && left == null && right == null;
2429
this.left = left;
2530
this.right = right;
31+
this.bytes = bytes;
32+
}
33+
34+
public boolean isBytes() {
35+
return bytes != null;
36+
}
37+
38+
public boolean isChildren() {
39+
return bytes == null;
2640
}
2741
}
2842

@@ -70,19 +84,8 @@ Rope withBinaryEncoding() {
7084
}
7185

7286
private ConcatRope withEncoding(Encoding encoding, CodeRange codeRange, int characterLength) {
73-
final ConcatChildren children = getChildrenOrNull();
74-
final ManagedRope left, right;
75-
final byte[] bytes;
76-
if (children != null) {
77-
left = children.left;
78-
right = children.right;
79-
bytes = null;
80-
} else {
81-
left = null;
82-
right = null;
83-
bytes = getRawBytes();
84-
}
85-
return new ConcatRope(left, right, encoding, codeRange, byteLength(), characterLength, bytes);
87+
final ConcatState state = getState();
88+
return new ConcatRope(state.left, state.right, encoding, codeRange, byteLength(), characterLength, state.bytes);
8689
}
8790

8891
@Override
@@ -93,32 +96,20 @@ protected byte[] getBytesSlow() {
9396
return out;
9497
}
9598

96-
public ConcatChildren getChildrenOrNull() {
99+
/** Access the state in a way that prevents race conditions. */
100+
public ConcatState getState() {
101+
if (this.bytes != null) {
102+
return new ConcatState(null, null, this.bytes);
103+
}
104+
97105
final ManagedRope left = this.left;
98106
final ManagedRope right = this.right;
99-
100107
if (left != null && right != null) {
101-
return new ConcatChildren(left, right);
102-
} else if (this.bytes != null) {
103-
return null;
104-
} else {
105-
CompilerDirectives.transferToInterpreterAndInvalidate();
106-
assert this.bytes != null;
107-
return null;
108+
return new ConcatState(left, right, null);
108109
}
109-
}
110110

111-
public byte[] getBytesOrNull() {
112-
final byte[] bytes = this.bytes;
113-
114-
if (bytes != null) {
115-
return bytes;
116-
} else if (this.left != null && this.right != null) {
117-
return null;
118-
} else {
119-
CompilerDirectives.transferToInterpreterAndInvalidate();
120-
assert this.bytes != null;
121-
return this.bytes;
122-
}
111+
CompilerDirectives.transferToInterpreterAndInvalidate();
112+
assert this.bytes != null;
113+
return new ConcatState(null, null, this.bytes);
123114
}
124115
}

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

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import org.truffleruby.SuppressFBWarnings;
3030
import org.truffleruby.core.array.ArrayUtils;
3131
import org.truffleruby.core.encoding.EncodingNodes;
32-
import org.truffleruby.core.rope.ConcatRope.ConcatChildren;
32+
import org.truffleruby.core.rope.ConcatRope.ConcatState;
3333
import org.truffleruby.core.rope.RopeNodesFactory.AreComparableRopesNodeGen;
3434
import org.truffleruby.core.rope.RopeNodesFactory.CompareRopesNodeGen;
3535
import org.truffleruby.core.rope.RopeNodesFactory.SetByteNodeGen;
@@ -806,9 +806,9 @@ protected Object debugPrintSubstringRope(SubstringRope rope, int currentLevel, b
806806
}
807807

808808
@TruffleBoundary
809-
@Specialization(guards = "bytes != null")
810-
protected Object debugPrintConcatRope(ConcatRope rope, int currentLevel, boolean printString,
811-
@Bind("rope.getBytesOrNull()") byte[] bytes) {
809+
@Specialization(guards = "state.isBytes()")
810+
protected Object debugPrintConcatRopeBytes(ConcatRope rope, int currentLevel, boolean printString,
811+
@Bind("rope.getState()") ConcatState state) {
812812
printPreamble(currentLevel);
813813

814814
System.err
@@ -826,12 +826,11 @@ protected Object debugPrintConcatRope(ConcatRope rope, int currentLevel, boolean
826826
}
827827

828828
@TruffleBoundary
829-
@Specialization(guards = "children != null")
830-
protected Object debugPrintConcatRope(ConcatRope rope, int currentLevel, boolean printString,
831-
@Bind("rope.getChildrenOrNull()") ConcatChildren children) {
829+
@Specialization(guards = "state.isChildren()")
830+
protected Object debugPrintConcatRopeChildren(ConcatRope rope, int currentLevel, boolean printString,
831+
@Bind("rope.getState()") ConcatState state) {
832832
printPreamble(currentLevel);
833833

834-
835834
System.err
836835
.println(StringUtils.format(
837836
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; E: %s)",
@@ -844,8 +843,8 @@ protected Object debugPrintConcatRope(ConcatRope rope, int currentLevel, boolean
844843
rope.getCodeRange(),
845844
rope.getEncoding()));
846845

847-
executeDebugPrint(children.left, currentLevel + 1, printString);
848-
executeDebugPrint(children.right, currentLevel + 1, printString);
846+
executeDebugPrint(state.left, currentLevel + 1, printString);
847+
executeDebugPrint(state.right, currentLevel + 1, printString);
849848

850849
return nil;
851850
}
@@ -1018,35 +1017,35 @@ protected int getByteRepeatingRope(RepeatingRope rope, int index,
10181017
return rope.getChild().getRawBytes()[index % rope.getChild().byteLength()] & 0xff;
10191018
}
10201019

1021-
@Specialization(guards = "children != null")
1020+
@Specialization(guards = "state.isChildren()")
10221021
protected int getByteConcatRope(ConcatRope rope, int index,
1023-
@Bind("rope.getChildrenOrNull()") ConcatChildren children,
1022+
@Bind("rope.getState()") ConcatState state,
10241023
@Cached ConditionProfile chooseLeftChildProfile,
10251024
@Cached ConditionProfile leftChildRawBytesNullProfile,
10261025
@Cached ConditionProfile rightChildRawBytesNullProfile,
10271026
@Cached ByteSlowNode byteSlowLeft,
10281027
@Cached ByteSlowNode byteSlowRight) {
1029-
if (chooseLeftChildProfile.profile(index < children.left.byteLength())) {
1030-
if (leftChildRawBytesNullProfile.profile(children.left.getRawBytes() == null)) {
1031-
return byteSlowLeft.execute(children.left, index) & 0xff;
1028+
if (chooseLeftChildProfile.profile(index < state.left.byteLength())) {
1029+
if (leftChildRawBytesNullProfile.profile(state.left.getRawBytes() == null)) {
1030+
return byteSlowLeft.execute(state.left, index) & 0xff;
10321031
}
10331032

1034-
return children.left.getRawBytes()[index] & 0xff;
1033+
return state.left.getRawBytes()[index] & 0xff;
10351034
}
10361035

1037-
if (rightChildRawBytesNullProfile.profile(children.right.getRawBytes() == null)) {
1038-
return byteSlowRight.execute(children.right, index - children.left.byteLength()) & 0xff;
1036+
if (rightChildRawBytesNullProfile.profile(state.right.getRawBytes() == null)) {
1037+
return byteSlowRight.execute(state.right, index - state.left.byteLength()) & 0xff;
10391038
}
10401039

1041-
return children.right.getRawBytes()[index - children.left.byteLength()] & 0xff;
1040+
return state.right.getRawBytes()[index - state.left.byteLength()] & 0xff;
10421041
}
10431042

10441043
// Necessary because getRawBytes() might return null, but then be populated and the children nulled
10451044
// before we get to run the other getByteConcatRope.
1046-
@Specialization(guards = "bytes != null")
1045+
@Specialization(guards = "state.isBytes()")
10471046
protected int getByteConcatRope(ConcatRope rope, int index,
1048-
@Bind("rope.getBytesOrNull()") byte[] bytes) {
1049-
return bytes[index] & 0xff;
1047+
@Bind("rope.getState()") ConcatState state) {
1048+
return state.bytes[index] & 0xff;
10501049
}
10511050
}
10521051

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

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import org.truffleruby.collections.IntStack;
4040
import org.truffleruby.core.Hashing;
4141
import org.truffleruby.core.encoding.EncodingManager;
42-
import org.truffleruby.core.rope.ConcatRope.ConcatChildren;
42+
import org.truffleruby.core.rope.ConcatRope.ConcatState;
4343
import org.truffleruby.core.rope.RopeNodesFactory.WithEncodingNodeGen;
4444
import org.truffleruby.core.string.StringAttributes;
4545
import org.truffleruby.core.string.StringOperations;
@@ -385,8 +385,8 @@ public static byte[] flattenBytes(Rope rope) {
385385
if (current instanceof ConcatRope) {
386386
final ConcatRope concatRope = (ConcatRope) current;
387387

388-
final ConcatChildren children = concatRope.getChildrenOrNull();
389-
if (children == null) {
388+
final ConcatState state = concatRope.getState();
389+
if (state.isBytes()) {
390390
// The rope got concurrently flattened between entering the iteration and reaching here,
391391
// restart the iteration from the top.
392392
workStack.push(concatRope);
@@ -395,26 +395,26 @@ public static byte[] flattenBytes(Rope rope) {
395395

396396
// In the absence of any SubstringRopes, we always take the full contents of the ConcatRope.
397397
if (substringLengths.isEmpty()) {
398-
workStack.push(children.right);
399-
workStack.push(children.left);
398+
workStack.push(state.right);
399+
workStack.push(state.left);
400400
} else {
401-
final int leftLength = children.left.byteLength();
401+
final int leftLength = state.left.byteLength();
402402

403403
// If we reach here, this ConcatRope is a descendant of a SubstringRope at some level. Based on
404404
// the currently calculated byte[] offset and the number of bytes to extract, determine which of
405405
// the ConcatRope's children we need to visit.
406406
if (byteOffset < leftLength) {
407407
if ((byteOffset + substringLengths.peek()) > leftLength) {
408-
workStack.push(children.right);
409-
workStack.push(children.left);
408+
workStack.push(state.right);
409+
workStack.push(state.left);
410410
} else {
411-
workStack.push(children.left);
411+
workStack.push(state.left);
412412
}
413413
} else {
414414
// If we can skip the left child entirely, we need to update the offset so it's accurate for
415415
// the right child as each child's starting point is 0.
416416
byteOffset -= leftLength;
417-
workStack.push(children.right);
417+
workStack.push(state.right);
418418
}
419419
}
420420
} else if (current instanceof SubstringRope) {
@@ -514,16 +514,16 @@ static byte getByteSlow(Rope rope, int index) {
514514
return rawBytes[index];
515515
} else if (rope instanceof ConcatRope) {
516516
final ConcatRope concatRope = (ConcatRope) rope;
517-
final ConcatChildren children = concatRope.getChildrenOrNull();
518-
if (children == null) {
517+
final ConcatState state = concatRope.getState();
518+
if (state.isBytes()) {
519519
// Rope got concurrently flattened.
520-
return concatRope.getRawBytes()[index];
520+
return state.bytes[index];
521521
}
522-
if (index < children.left.byteLength()) {
523-
rope = children.left;
522+
if (index < state.left.byteLength()) {
523+
rope = state.left;
524524
} else {
525-
rope = children.right;
526-
index -= children.left.byteLength();
525+
rope = state.right;
526+
index -= state.left.byteLength();
527527
}
528528
} else if (rope instanceof SubstringRope) {
529529
final SubstringRope substringRope = (SubstringRope) rope;
@@ -586,13 +586,13 @@ class Params {
586586
workStack.push(new Params(child, startingHashCode, newOffset, length, false));
587587
} else if (rope instanceof ConcatRope) {
588588
final ConcatRope concatRope = (ConcatRope) rope;
589-
final ConcatChildren children = concatRope.getChildrenOrNull();
590-
if (children == null) {
589+
final ConcatState state = concatRope.getState();
590+
if (state.isBytes()) {
591591
// Rope got concurrently flattened.
592-
resultHash = Hashing.stringHash(bytes, startingHashCode, offset, length);
592+
resultHash = Hashing.stringHash(state.bytes, startingHashCode, offset, length);
593593
} else {
594-
final Rope left = children.left;
595-
final Rope right = children.right;
594+
final Rope left = state.left;
595+
final Rope right = state.right;
596596
final int leftLength = left.byteLength();
597597

598598
if (offset >= leftLength) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import org.truffleruby.builtins.CoreMethod;
1515
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
1616
import org.truffleruby.builtins.CoreModule;
17-
import org.truffleruby.core.rope.ConcatRope.ConcatChildren;
17+
import org.truffleruby.core.rope.ConcatRope.ConcatState;
1818
import org.truffleruby.core.string.RubyString;
1919
import org.truffleruby.core.string.StringNodes;
2020
import org.truffleruby.core.string.StringOperations;
@@ -120,10 +120,10 @@ private static String getStructure(LeafRope rope) {
120120
}
121121

122122
private static String getStructure(ConcatRope rope) {
123-
final ConcatChildren children = rope.getChildrenOrNull();
124-
return children != null
125-
? "(" + getStructure(children.left) + " + " + getStructure(children.right) + ")"
126-
: "\"" + rope.toString() + "\" (flat concat)";
123+
final ConcatState state = rope.getState();
124+
return state.isChildren()
125+
? "(" + getStructure(state.left) + " + " + getStructure(state.right) + ")"
126+
: "(\"flat concat rope\"; " + rope.toString() + ")";
127127
}
128128

129129
private static String getStructure(SubstringRope rope) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@
125125
import org.truffleruby.core.regexp.RubyRegexp;
126126
import org.truffleruby.core.rope.CodeRange;
127127
import org.truffleruby.core.rope.ConcatRope;
128-
import org.truffleruby.core.rope.ConcatRope.ConcatChildren;
128+
import org.truffleruby.core.rope.ConcatRope.ConcatState;
129129
import org.truffleruby.core.rope.LeafRope;
130130
import org.truffleruby.core.rope.NativeRope;
131131
import org.truffleruby.core.rope.RepeatingRope;
@@ -5199,10 +5199,10 @@ private SearchResult searchForSingleByteOptimizableDescendantSlow(Rope base, int
51995199
} else if (base instanceof ConcatRope) {
52005200
final ConcatRope concatRope = (ConcatRope) base;
52015201

5202-
final ConcatChildren children = concatRope.getChildrenOrNull();
5203-
if (children != null) {
5204-
final Rope left = children.left;
5205-
final Rope right = children.right;
5202+
final ConcatState state = concatRope.getState();
5203+
if (state.isChildren()) {
5204+
final Rope left = state.left;
5205+
final Rope right = state.right;
52065206
if (index + characterLength <= left.characterLength()) {
52075207
return searchForSingleByteOptimizableDescendantSlow(left, index, characterLength);
52085208
} else if (index >= left.characterLength()) {

0 commit comments

Comments
 (0)