Skip to content

Commit ec62fae

Browse files
author
Nicolas Laurent
committed
[GR-26661] Release memory for children ropes when a concat rope becomes flattened
PullRequest: truffleruby/2265
2 parents 19f0e6a + 7b3fe84 commit ec62fae

13 files changed

+239
-120
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
package org.truffleruby.core.rope;
1212

13+
import com.oracle.truffle.api.profiles.ConditionProfile;
1314
import org.jcodings.Encoding;
1415

1516
import com.oracle.truffle.api.CompilerDirectives;
@@ -23,12 +24,12 @@ public AsciiOnlyLeafRope(byte[] bytes, Encoding encoding) {
2324
}
2425

2526
@Override
26-
Rope withEncoding7bit(Encoding newEncoding) {
27+
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
2728
return new AsciiOnlyLeafRope(getRawBytes(), newEncoding);
2829
}
2930

3031
@Override
31-
Rope withBinaryEncoding() {
32+
Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
3233
CompilerDirectives.transferToInterpreterAndInvalidate();
3334
throw new UnsupportedOperationException("Must only be called for CR_VALID Strings");
3435
}

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

Lines changed: 86 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,68 +9,125 @@
99
*/
1010
package org.truffleruby.core.rope;
1111

12+
import com.oracle.truffle.api.CompilerAsserts;
13+
import com.oracle.truffle.api.CompilerDirectives;
14+
import com.oracle.truffle.api.CompilerDirectives.ValueType;
15+
import com.oracle.truffle.api.profiles.ConditionProfile;
1216
import org.jcodings.Encoding;
1317
import org.jcodings.specific.ASCIIEncoding;
1418

1519
public class ConcatRope extends ManagedRope {
1620

17-
private final ManagedRope left;
18-
private final ManagedRope right;
21+
/** Wrapper for the current state of the concat rope, including null children and a a byte array, or a null byte
22+
* array and the children. Accessing the state through {@link #getState()} avoids race conditions. */
23+
@ValueType
24+
public static class ConcatState {
25+
public final ManagedRope left, right;
26+
public final byte[] bytes;
27+
28+
public ConcatState(ManagedRope left, ManagedRope right, byte[] bytes) {
29+
assert bytes == null && left != null && right != null || bytes != null && left == null && right == null;
30+
this.left = left;
31+
this.right = right;
32+
this.bytes = bytes;
33+
}
34+
35+
public boolean isBytes() {
36+
return bytes != null;
37+
}
38+
39+
public boolean isChildren() {
40+
return bytes == null;
41+
}
42+
}
43+
44+
private ManagedRope left;
45+
private ManagedRope right;
1946

2047
public ConcatRope(
2148
ManagedRope left,
2249
ManagedRope right,
2350
Encoding encoding,
2451
CodeRange codeRange) {
25-
this(left, right, encoding, codeRange, left.characterLength() + right.characterLength(), null);
52+
this(
53+
left,
54+
right,
55+
encoding,
56+
codeRange,
57+
left.byteLength() + right.byteLength(),
58+
left.characterLength() + right.characterLength(),
59+
null);
2660
}
2761

2862
private ConcatRope(
2963
ManagedRope left,
3064
ManagedRope right,
3165
Encoding encoding,
3266
CodeRange codeRange,
67+
int byteLength,
3368
int characterLength,
3469
byte[] bytes) {
35-
super(
36-
encoding,
37-
codeRange,
38-
left.byteLength() + right.byteLength(),
39-
characterLength,
40-
bytes);
70+
super(encoding, codeRange, byteLength, characterLength, bytes);
4171
this.left = left;
4272
this.right = right;
4373
}
4474

4575
@Override
46-
Rope withEncoding7bit(Encoding newEncoding) {
76+
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
4777
assert getCodeRange() == CodeRange.CR_7BIT;
48-
return new ConcatRope(
49-
getLeft(),
50-
getRight(),
51-
newEncoding,
52-
CodeRange.CR_7BIT,
53-
characterLength(),
54-
getRawBytes());
78+
return withEncoding(newEncoding, CodeRange.CR_7BIT, characterLength(), bytesNotNull);
5579
}
5680

5781
@Override
58-
Rope withBinaryEncoding() {
82+
Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
5983
assert getCodeRange() == CodeRange.CR_VALID;
60-
return new ConcatRope(
61-
getLeft(),
62-
getRight(),
63-
ASCIIEncoding.INSTANCE,
64-
CodeRange.CR_VALID,
65-
byteLength(),
66-
getRawBytes());
84+
return withEncoding(ASCIIEncoding.INSTANCE, CodeRange.CR_VALID, byteLength(), bytesNotNull);
6785
}
6886

69-
public ManagedRope getLeft() {
70-
return left;
87+
private ConcatRope withEncoding(Encoding encoding, CodeRange codeRange, int characterLength,
88+
ConditionProfile bytesNotNull) {
89+
final ConcatState state = getState(bytesNotNull);
90+
return new ConcatRope(state.left, state.right, encoding, codeRange, byteLength(), characterLength, state.bytes);
7191
}
7292

73-
public ManagedRope getRight() {
74-
return right;
93+
@Override
94+
protected byte[] getBytesSlow() {
95+
bytes = RopeOperations.flattenBytes(this);
96+
left = null;
97+
right = null;
98+
return bytes;
99+
}
100+
101+
/** Access the state in a way that prevents race conditions.
102+
*
103+
* <p>
104+
* This version is not allowed in compiled code, use {@link #getState(ConditionProfile)} there instead. */
105+
public ConcatState getState() {
106+
CompilerAsserts.neverPartOfCompilation("Use #getState(ConditionProfile) instead.");
107+
return getState(ConditionProfile.getUncached());
108+
}
109+
110+
/** Access the state in a way that prevents race conditions.
111+
*
112+
* <p>
113+
* Outside compiled code, you can use {@link #getState()}. */
114+
public ConcatState getState(ConditionProfile bytesNotNull) {
115+
if (bytesNotNull.profile(this.bytes != null)) {
116+
return new ConcatState(null, null, this.bytes);
117+
}
118+
119+
final ManagedRope left = this.left;
120+
final ManagedRope right = this.right;
121+
if (left != null && right != null) {
122+
return new ConcatState(left, right, null);
123+
}
124+
125+
CompilerDirectives.transferToInterpreterAndInvalidate();
126+
if (this.bytes != null) {
127+
throw CompilerDirectives
128+
.shouldNotReachHere("our assumptions about reordering and memory barriers seem incorrect");
129+
}
130+
131+
return new ConcatState(null, null, this.bytes);
75132
}
76133
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
package org.truffleruby.core.rope;
1212

13+
import com.oracle.truffle.api.profiles.ConditionProfile;
1314
import org.jcodings.Encoding;
1415

1516
import com.oracle.truffle.api.CompilerDirectives;
@@ -23,13 +24,13 @@ public InvalidLeafRope(byte[] bytes, Encoding encoding, int characterLength) {
2324
}
2425

2526
@Override
26-
Rope withEncoding7bit(Encoding newEncoding) {
27+
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
2728
CompilerDirectives.transferToInterpreterAndInvalidate();
2829
throw new UnsupportedOperationException("Must only be called for ASCII-only Strings");
2930
}
3031

3132
@Override
32-
Rope withBinaryEncoding() {
33+
Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
3334
CompilerDirectives.transferToInterpreterAndInvalidate();
3435
throw new UnsupportedOperationException("Must only be called for CR_VALID Strings");
3536
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.truffleruby.core.rope;
1111

1212
import com.oracle.truffle.api.CompilerDirectives;
13+
import com.oracle.truffle.api.profiles.ConditionProfile;
1314
import org.jcodings.Encoding;
1415
import org.jcodings.specific.USASCIIEncoding;
1516

@@ -52,13 +53,13 @@ private static int length(int value) {
5253
}
5354

5455
@Override
55-
Rope withEncoding7bit(Encoding newEncoding) {
56+
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
5657
assert getCodeRange() == CodeRange.CR_7BIT;
5758
return new LazyIntRope(value, newEncoding, length(value));
5859
}
5960

6061
@Override
61-
Rope withBinaryEncoding() {
62+
Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
6263
CompilerDirectives.transferToInterpreterAndInvalidate();
6364
throw new UnsupportedOperationException("Must only be called for CR_VALID Strings");
6465
}

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

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

12+
import com.oracle.truffle.api.profiles.ConditionProfile;
1213
import org.jcodings.Encoding;
1314
import org.jcodings.specific.ASCIIEncoding;
1415
import org.truffleruby.core.FinalizationService;
@@ -188,12 +189,12 @@ public String toString() {
188189
}
189190

190191
@Override
191-
Rope withEncoding7bit(Encoding newEncoding) {
192+
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
192193
return withEncoding(newEncoding);
193194
}
194195

195196
@Override
196-
Rope withBinaryEncoding() {
197+
Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
197198
return withEncoding(ASCIIEncoding.INSTANCE);
198199
}
199200

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
package org.truffleruby.core.rope;
1212

13+
import com.oracle.truffle.api.profiles.ConditionProfile;
1314
import org.jcodings.Encoding;
1415
import org.jcodings.specific.ASCIIEncoding;
1516

@@ -31,13 +32,13 @@ public RepeatingRope(ManagedRope child, int times, int byteLength) {
3132
}
3233

3334
@Override
34-
Rope withEncoding7bit(Encoding newEncoding) {
35+
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
3536
assert getCodeRange() == CodeRange.CR_7BIT;
3637
return new RepeatingRope((ManagedRope) RopeOperations.withEncoding(child, newEncoding), times, byteLength());
3738
}
3839

3940
@Override
40-
Rope withBinaryEncoding() {
41+
Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
4142
assert getCodeRange() == CodeRange.CR_VALID;
4243
return new RepeatingRope(
4344
(ManagedRope) RopeOperations.withEncoding(child, ASCIIEncoding.INSTANCE),

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import java.util.Arrays;
1313

14+
import com.oracle.truffle.api.profiles.ConditionProfile;
1415
import org.jcodings.Encoding;
1516

1617
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -38,11 +39,11 @@ protected Rope(Encoding encoding, int byteLength, byte[] bytes) {
3839

3940
/** Only used internally by WithEncodingNode. Returns a Rope with the given Encoding. Both the original and new
4041
* Encodings must be ASCII-compatible and the rope must be {@link #isAsciiOnly()}. */
41-
abstract Rope withEncoding7bit(Encoding newEncoding);
42+
abstract Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull);
4243

4344
/** Only used internally by WithEncodingNode. Returns a Rope with the BINARY Encoding. The original Encoding must be
4445
* ASCII-compatible and {@link #getCodeRange()} must be {@link CodeRange#CR_VALID} to call this. */
45-
abstract Rope withBinaryEncoding();
46+
abstract Rope withBinaryEncoding(ConditionProfile bytesNotNull);
4647

4748
public abstract int characterLength();
4849

0 commit comments

Comments
 (0)