Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 81bfe6a11704
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: ddbcb76e450b
Choose a head ref
  • 11 commits
  • 8 files changed
  • 1 contributor

Commits on Aug 26, 2016

  1. [Truffle] Sped up making a RepeatingRope from a RopeBuffer.

    MRI and JRuby both use this approach of expanding the copied region. We must have just missed it when doing the initial implementation.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    9efb699 View commit details
  2. [Truffle] Added a faster path for fetching the bytes of a RepeatingRo…

    …pe whose child has its bytes populated.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    a6c7d33 View commit details
  3. [Truffle] Added a faster path for fetching the bytes of a SubstringRo…

    …pe whose child has its bytes populated.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    10726d2 View commit details
  4. Copy the full SHA
    b5c1c39 View commit details
  5. Copy the full SHA
    732dfac View commit details
  6. [Truffle] Fixed String#+ for RopeBuffers.

    Without an explicit dup of the RopeBuffer, we'd mutate the receiver's bytes.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    faa08ec View commit details
  7. [Truffle] Fixed String#bytesize for RopeBuffers.

    While we could fix this by allowing Rope#byteLength to be overrideable, we don't want to make that call virtual.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    441873b View commit details
  8. [Truffle] Fixed String#replace for RopeBuffers.

    Without duping the original RopeBuffer any modifications to the resulting string will also be visible in the source string.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    fbcfbdb View commit details
  9. [Truffle] Simplified the guard checking if a substring would simply b…

    …e the same as its child.
    
    The old calculation took the offset into account and I think this was a bug. A non-zero offset would trivially ensure the substring and its child were not the same. Since the byte length of the substring is bounded by the byte length of the child (enforced at a higher level), we can reduce the check to simply seeing if the substring has the same byte length as the child.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    ce464e4 View commit details
  10. [Truffle] Avoid reprocessing a complex rope when getting the bytes fo…

    …r a RepeatingRope by flattening its child first.
    nirvdrum committed Aug 26, 2016
    Copy the full SHA
    1703f16 View commit details
  11. Copy the full SHA
    ddbcb76 View commit details
Original file line number Diff line number Diff line change
@@ -28,6 +28,28 @@ public Rope withEncoding(Encoding newEncoding, CodeRange newCodeRange) {
return new RepeatingRope(child.withEncoding(newEncoding, newCodeRange), times);
}

@Override
protected byte[] getBytesSlow() {
if (child.getRawBytes() != null) {
final byte[] childBytes = child.getRawBytes();
int len = childBytes.length * times;
final byte[] ret = new byte[len];

int n = childBytes.length;

System.arraycopy(childBytes, 0, ret, 0, n);
while (n <= len / 2) {
System.arraycopy(ret, 0, ret, n, n);
n *= 2;
}
System.arraycopy(ret, 0, ret, n, len - n);

return ret;
}

return super.getBytesSlow();
}

@Override
protected byte getByteSlow(int index) {
return child.getByteSlow(index % child.byteLength());
6 changes: 5 additions & 1 deletion truffle/src/main/java/org/jruby/truffle/core/rope/Rope.java
Original file line number Diff line number Diff line change
@@ -58,12 +58,16 @@ public final byte[] getRawBytes() {

public final byte[] getBytes() {
if (bytes == null) {
bytes = RopeOperations.flattenBytes(this);
bytes = getBytesSlow();
}

return bytes;
}

protected byte[] getBytesSlow() {
return RopeOperations.flattenBytes(this);
}

public final byte[] getBytesCopy() {
return getBytes().clone();
}
Original file line number Diff line number Diff line change
@@ -56,4 +56,7 @@ public String toString() {
return byteList.toString();
}

public RopeBuffer dup() {
return new RopeBuffer(byteList.dup(), getCodeRange(), isSingleByteOptimizable(), characterLength());
}
}
32 changes: 21 additions & 11 deletions truffle/src/main/java/org/jruby/truffle/core/rope/RopeNodes.java
Original file line number Diff line number Diff line change
@@ -104,26 +104,26 @@ public Rope substringOneByte(Rope base, int offset, int byteLength,
return RopeOperations.withEncodingVerySlow(RopeConstants.ASCII_8BIT_SINGLE_BYTE_ROPES[index], base.getEncoding());
}

@Specialization(guards = { "byteLength > 1", "sameAsBase(base, offset, byteLength)" })
@Specialization(guards = { "byteLength > 1", "sameAsBase(base, byteLength)" })
public Rope substringSameAsBase(Rope base, int offset, int byteLength) {
return base;
}

@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, offset, byteLength)" })
@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, byteLength)" })
public Rope substringLeafRope(LeafRope base, int offset, int byteLength,
@Cached("createBinaryProfile()") ConditionProfile is7BitProfile,
@Cached("createBinaryProfile()") ConditionProfile isBinaryStringProfile) {
return makeSubstring(base, offset, byteLength, is7BitProfile, isBinaryStringProfile);
}

@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, offset, byteLength)" })
@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, byteLength)" })
public Rope substringSubstringRope(SubstringRope base, int offset, int byteLength,
@Cached("createBinaryProfile()") ConditionProfile is7BitProfile,
@Cached("createBinaryProfile()") ConditionProfile isBinaryStringProfile) {
return makeSubstring(base.getChild(), offset + base.getOffset(), byteLength, is7BitProfile, isBinaryStringProfile);
}

@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, offset, byteLength)" })
@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, byteLength)" })
public Rope substringRepeatingRope(RepeatingRope base, int offset, int byteLength,
@Cached("createBinaryProfile()") ConditionProfile is7BitProfile,
@Cached("createBinaryProfile()") ConditionProfile isBinaryStringProfile,
@@ -139,14 +139,14 @@ public Rope substringRepeatingRope(RepeatingRope base, int offset, int byteLengt
return makeSubstring(base, offset, byteLength, is7BitProfile, isBinaryStringProfile);
}

@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, offset, byteLength)" })
@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, byteLength)" })
public Rope substringLazyRope(LazyRope base, int offset, int byteLength,
@Cached("createBinaryProfile()") ConditionProfile is7BitProfile,
@Cached("createBinaryProfile()") ConditionProfile isBinaryStringProfile) {
return makeSubstring(base, offset, byteLength, is7BitProfile, isBinaryStringProfile);
}

@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, offset, byteLength)" })
@Specialization(guards = { "byteLength > 1", "!sameAsBase(base, byteLength)" })
public Rope substringConcatRope(ConcatRope base, int offset, int byteLength,
@Cached("createBinaryProfile()") ConditionProfile is7BitProfile,
@Cached("createBinaryProfile()") ConditionProfile isBinaryStringProfile) {
@@ -220,8 +220,10 @@ private Rope makeSubstringNon7Bit(Rope base, int offset, int byteLength) {
}
}

protected static boolean sameAsBase(Rope base, int offset, int byteLength) {
return (byteLength - offset) == base.byteLength();
protected static boolean sameAsBase(Rope base, int byteLength) {
// A SubstringRope's byte length is not allowed to be larger than its child. Thus, if it has the same
// byte length as its child, it must be logically equivalent to the child.
return byteLength == base.byteLength();
}

}
@@ -524,11 +526,19 @@ public Rope repeatOne(Rope base, int times,
@Specialization(guards = "times > 1")
public Rope multiplyBuffer(RopeBuffer base, int times) {
final ByteList inputBytes = base.getByteList();
final ByteList outputBytes = new ByteList(inputBytes.realSize() * times);
int len = inputBytes.realSize() * times;
final ByteList outputBytes = new ByteList(len);
outputBytes.realSize(len);

for (int i = 0; i < times; i++) {
outputBytes.append(inputBytes);
int n = inputBytes.realSize();

System.arraycopy(inputBytes.unsafeBytes(), inputBytes.begin(), outputBytes.unsafeBytes(), 0, n);
while (n <= len / 2) {
System.arraycopy(outputBytes.unsafeBytes(), 0, outputBytes.unsafeBytes(), n, n);
n *= 2;
}
System.arraycopy(outputBytes.unsafeBytes(), 0, outputBytes.unsafeBytes(), n, len - n);


outputBytes.setEncoding(inputBytes.getEncoding());

Original file line number Diff line number Diff line change
@@ -417,9 +417,10 @@ public static byte[] flattenBytes(Rope rope) {
loopCount++;
}

// TODO (nirvdrum 06-Apr-16) Rather than process the same child over and over, there may be opportunity to re-use the results from whole sections.
// TODO (nirvdrum 25-Aug-2016): Flattening the rope with CR_VALID will cause a character length recalculation, even though we already know what it is. That operation should be made more optimal.
final Rope flattenedChild = flatten(repeatingRope.getChild());
for (int i = 0; i < loopCount; i++) {
workStack.push(repeatingRope.getChild());
workStack.push(flattenedChild);
}
}
} else {
Original file line number Diff line number Diff line change
@@ -33,6 +33,8 @@ private SubstringRope(Rope child, Encoding encoding, boolean singleByteOptimizab
super(encoding, codeRange, singleByteOptimizable, byteLength, characterLength, child.depth() + 1, null);
this.child = child;
this.offset = offset;

assert byteLength <= child.byteLength();
}

@Override
@@ -45,6 +47,19 @@ public Rope withEncoding(Encoding newEncoding, CodeRange newCodeRange) {
return new SubstringRope(getChild(), newEncoding, getChild().isSingleByteOptimizable(), getOffset(), byteLength(), characterLength(), newCodeRange);
}

@Override
protected byte[] getBytesSlow() {
if (child.getRawBytes() != null) {
final byte[] ret = new byte[byteLength()];

System.arraycopy(child.getRawBytes(), offset, ret, 0, byteLength());

return ret;
}

return RopeOperations.extractRange(child, offset, byteLength());
}

@Override
public byte getByteSlow(int index) {
return child.getByteSlow(index + offset);
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.profiles.BranchProfile;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.source.SourceSection;
import jnr.constants.platform.Errno;
import org.jruby.truffle.Layouts;
@@ -51,6 +52,7 @@
import org.jruby.truffle.builtins.PrimitiveArrayArgumentsNode;
import org.jruby.truffle.core.exception.ExceptionOperations;
import org.jruby.truffle.core.rope.Rope;
import org.jruby.truffle.core.rope.RopeBuffer;
import org.jruby.truffle.core.string.StringOperations;
import org.jruby.truffle.language.control.RaiseException;
import org.jruby.truffle.language.objects.AllocateObjectNode;
@@ -87,22 +89,35 @@ public DynamicObject allocate(DynamicObject classToAllocate) {
public static abstract class IOBufferUnshiftPrimitiveNode extends PrimitiveArrayArgumentsNode {

@Specialization(guards = "isRubyString(string)")
public int unshift(VirtualFrame frame, DynamicObject ioBuffer, DynamicObject string, int startPosition) {
public int unshift(VirtualFrame frame, DynamicObject ioBuffer, DynamicObject string, int startPosition,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
Layouts.IO_BUFFER.setWriteSynced(ioBuffer, false);

final Rope rope = StringOperations.rope(string);
int stringSize = rope.byteLength() - startPosition;
final int usedSpace = Layouts.IO_BUFFER.getUsed(ioBuffer);
final int availableSpace = IOBUFFER_SIZE - usedSpace;

final byte[] bytes;
int stringSize;

if (ropeBufferProfile.profile(rope instanceof RopeBuffer)) {
final ByteList byteList = ((RopeBuffer) rope).getByteList();

bytes = byteList.unsafeBytes();
stringSize = byteList.realSize() - startPosition;
} else {
bytes = rope.getBytes();
stringSize = rope.byteLength() - startPosition;
}

if (stringSize > availableSpace) {
stringSize = availableSpace;
}

ByteList storage = Layouts.BYTE_ARRAY.getBytes(Layouts.IO_BUFFER.getStorage(ioBuffer));

// Data is copied here - can we do something COW?
System.arraycopy(rope.getBytes(), startPosition, storage.getUnsafeBytes(), storage.begin() + usedSpace, stringSize);
// TODO (nirvdrum 08-24-16): Data is copied here - can we do something COW?
System.arraycopy(bytes, startPosition, storage.getUnsafeBytes(), storage.begin() + usedSpace, stringSize);

Layouts.IO_BUFFER.setUsed(ioBuffer, usedSpace + stringSize);

Original file line number Diff line number Diff line change
@@ -200,25 +200,32 @@ public AddNode(RubyContext context, SourceSection sourceSection) {
}

@Specialization(guards = {"isRubyString(other)", "getEncoding(string) == getEncoding(other)"})
public DynamicObject addSameEncoding(DynamicObject string, DynamicObject other) {
return add(string, other, getEncoding(string));
public DynamicObject addSameEncoding(DynamicObject string, DynamicObject other,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
return add(string, other, getEncoding(string), ropeBufferProfile);
}

@Specialization(guards = {"isRubyString(other)", "getEncoding(string) != getEncoding(other)", "isUTF8AndUSASCII(string, other)"})
public DynamicObject addUTF8AndUSASCII(DynamicObject string, DynamicObject other) {
return add(string, other, UTF8Encoding.INSTANCE);
public DynamicObject addUTF8AndUSASCII(DynamicObject string, DynamicObject other,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
return add(string, other, UTF8Encoding.INSTANCE, ropeBufferProfile);
}

@Specialization(guards = {"isRubyString(other)", "getEncoding(string) != getEncoding(other)", "!isUTF8AndUSASCII(string, other)"})
public DynamicObject addDifferentEncodings(DynamicObject string, DynamicObject other) {
public DynamicObject addDifferentEncodings(DynamicObject string, DynamicObject other,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
final Encoding enc = checkEncodingNode.executeCheckEncoding(string, other);
return add(string, other, enc);
return add(string, other, enc, ropeBufferProfile);
}

private DynamicObject add(DynamicObject string, DynamicObject other, Encoding encoding) {
final Rope left = rope(string);
private DynamicObject add(DynamicObject string, DynamicObject other, Encoding encoding, ConditionProfile ropeBufferProfile) {
Rope left = rope(string);
final Rope right = rope(other);

if (ropeBufferProfile.profile(left instanceof RopeBuffer)) {
left = ((RopeBuffer) left).dup();
}

final Rope concatRope = makeConcatNode.executeMake(left, right, encoding);

final DynamicObject ret = Layouts.STRING.createString(coreLibrary().getStringFactory(), concatRope);
@@ -706,7 +713,13 @@ public DynamicObject bytes(VirtualFrame frame, DynamicObject string, DynamicObje
public abstract static class ByteSizeNode extends CoreMethodArrayArgumentsNode {

@Specialization
public int byteSize(DynamicObject string) {
public int byteSize(DynamicObject string, @Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
final Rope rope = rope(string);

if (ropeBufferProfile.profile(rope instanceof RopeBuffer)) {
return ((RopeBuffer) rope).getByteList().realSize();
}

return rope(string).byteLength();
}

@@ -1555,8 +1568,15 @@ public DynamicObject replaceStringIsSameAsOther(DynamicObject string, DynamicObj


@Specialization(guards = { "string != other", "isRubyString(other)" })
public DynamicObject replace(DynamicObject string, DynamicObject other) {
StringOperations.setRope(string, rope(other));
public DynamicObject replace(DynamicObject string, DynamicObject other,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
final Rope rope = rope(other);

if (ropeBufferProfile.profile(rope instanceof RopeBuffer)) {
StringOperations.setRope(string, ((RopeBuffer) rope).dup());
} else {
StringOperations.setRope(string, rope);
}

return string;
}
@@ -4189,17 +4209,17 @@ private SearchResult searchForSingleByteOptimizableDescendant(Rope base, int ind
final Rope left = concatRope.getLeft();
final Rope right = concatRope.getRight();

if (index < left.byteLength()) {
if (index < left.characterLength()) {
return searchForSingleByteOptimizableDescendant(left, index, length);
} else if (index >= left.byteLength()) {
return searchForSingleByteOptimizableDescendant(right, index - left.byteLength(), length);
} else if (index >= left.characterLength()) {
return searchForSingleByteOptimizableDescendant(right, index - left.characterLength(), length);
} else {
return new SearchResult(index, concatRope);
}
} else if (base instanceof RepeatingRope) {
final RepeatingRope repeatingRope = (RepeatingRope) base;

if (index + length < repeatingRope.getChild().byteLength()) {
if (index + length < repeatingRope.getChild().characterLength()) {
return searchForSingleByteOptimizableDescendant(repeatingRope.getChild(), index, length);
} else {
return new SearchResult(index, repeatingRope);