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: dc3dc8562a19
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 622897f5ceef
Choose a head ref
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Aug 30, 2016

  1. Copy the full SHA
    900d8ac View commit details
  2. [Truffle] Reworked String#+ with a RopeBuffer receiver to avoid an un…

    …necessary memory allocation and copy.
    
    The previous implementation duped the RopeBuffer (1 allocation and copy) and then let the MakeConcatRope node expand the RopeBuffer's ByteList by appending the other string's nodes. Unless the other string was empty, this would force the ByteList's backing store to grow (second allocation and copy to temp buffer). This new implementation allocates the resulting ByteList immediately and just copy's the receiver and argument bytes into it.
    nirvdrum committed Aug 30, 2016
    Copy the full SHA
    622897f View commit details
Showing with 62 additions and 23 deletions.
  1. +9 −0 truffle/src/main/java/org/jruby/truffle/core/string/StringGuards.java
  2. +53 −23 truffle/src/main/java/org/jruby/truffle/core/string/StringNodes.java
Original file line number Diff line number Diff line change
@@ -15,8 +15,11 @@
import org.jruby.truffle.Layouts;
import org.jruby.truffle.core.rope.CodeRange;
import org.jruby.truffle.core.rope.Rope;
import org.jruby.truffle.core.rope.RopeBuffer;
import org.jruby.truffle.language.RubyGuards;

import static org.jruby.truffle.core.string.StringOperations.rope;

public class StringGuards {

public static boolean isSingleByteOptimizable(DynamicObject string) {
@@ -65,4 +68,10 @@ public static boolean isBinaryString(DynamicObject string) {
assert RubyGuards.isRubyString(string);
return StringOperations.encoding(string) == ASCIIEncoding.INSTANCE;
}

public static boolean isRopeBuffer(DynamicObject string) {
assert RubyGuards.isRubyString(string);

return rope(string) instanceof RopeBuffer;
}
}
Original file line number Diff line number Diff line change
@@ -183,6 +183,7 @@ public DynamicObject allocate(DynamicObject rubyClass) {
@NodeChild(type = RubyNode.class, value = "string"),
@NodeChild(type = RubyNode.class, value = "other")
})
@ImportStatic(StringGuards.class)
public abstract static class AddNode extends CoreMethodNode {

@Child private EncodingNodes.CheckEncodingNode checkEncodingNode;
@@ -200,33 +201,59 @@ public AddNode(RubyContext context, SourceSection sourceSection) {
return ToStrNodeGen.create(null, null, other);
}

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

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

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

private DynamicObject add(DynamicObject string, DynamicObject other, Encoding encoding, ConditionProfile ropeBufferProfile) {
Rope left = rope(string);
@Specialization(guards = { "isRopeBuffer(string)", "is7Bit(string)", "is7Bit(other)" })
public DynamicObject addRopeBuffer(DynamicObject string, DynamicObject other,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
final Encoding enc = checkEncodingNode.executeCheckEncoding(string, other);

final RopeBuffer left = (RopeBuffer) rope(string);
final ByteList leftByteList = left.getByteList();
final Rope right = rope(other);

if (ropeBufferProfile.profile(left instanceof RopeBuffer)) {
left = ((RopeBuffer) left).dup();
final ByteList concatByteList;
if (ropeBufferProfile.profile(right instanceof RopeBuffer)) {
concatByteList = StringSupport.addByteLists(leftByteList, ((RopeBuffer) right).getByteList());
} else {
// Taken from org.jruby.util.StringSupport.addByteLists.

final int newLength = leftByteList.realSize() + right.byteLength();
concatByteList = new ByteList(newLength);
concatByteList.realSize(newLength);
System.arraycopy(leftByteList.unsafeBytes(), leftByteList.begin(), concatByteList.unsafeBytes(), 0, leftByteList.realSize());
System.arraycopy(right.getBytes(), 0, concatByteList.unsafeBytes(), leftByteList.realSize(), right.byteLength());
}

concatByteList.setEncoding(enc);

final RopeBuffer concatRope = new RopeBuffer(concatByteList, left.getCodeRange(), left.isSingleByteOptimizable(), concatByteList.realSize());
final DynamicObject ret = Layouts.STRING.createString(coreLibrary().getStringFactory(), concatRope);

taintResultNode.maybeTaint(string, ret);
taintResultNode.maybeTaint(other, ret);

return ret;
}

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

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

final DynamicObject ret = Layouts.STRING.createString(coreLibrary().getStringFactory(), concatRope);
@@ -1348,8 +1375,15 @@ public Object initializeCopySelfIsSameAsFrom(DynamicObject self, DynamicObject f


@Specialization(guards = { "self != from", "isRubyString(from)" })
public Object initializeCopy(DynamicObject self, DynamicObject from) {
StringOperations.setRope(self, rope(from));
public Object initializeCopy(DynamicObject self, DynamicObject from,
@Cached("createBinaryProfile()") ConditionProfile ropeBufferProfile) {
final Rope rope = rope(from);

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

return self;
}
@@ -1782,6 +1816,7 @@ private ByteList dumpCommon(DynamicObject string) {
@NodeChild(type = RubyNode.class, value = "index"),
@NodeChild(type = RubyNode.class, value = "value")
})
@ImportStatic(StringGuards.class)
public abstract static class SetByteNode extends CoreMethodNode {

@Child private RopeNodes.MakeConcatNode composedMakeConcatNode;
@@ -1836,11 +1871,6 @@ public int setByteRopeBuffer(DynamicObject string, int index, int value) {
return value;
}

protected boolean isRopeBuffer(DynamicObject string) {
assert RubyGuards.isRubyString(string);

return rope(string) instanceof RopeBuffer;
}
}

@CoreMethod(names = {"size", "length"})