Skip to content

Commit

Permalink
[Truffle] All rope bytes will start at offset 0.
Browse files Browse the repository at this point in the history
The code that was here was placeholder code for algorithms converted from ByteList. The idea was something like SubstringRope could share its raw bytes with its child, and then alter its 'begin' value to demarcate its real portion of the underlying byte array. While there may be real benefits to that level of byte sharing, it complicates all code that must work with the bytes. And we've already assumed the 'begin' value is 0 in several places.

Should we wish to revisit the byte sharing, it's probably best done in the context of a specialized node for whatever operation.
nirvdrum committed Apr 14, 2016
1 parent c22dfd0 commit 7f8d612
Showing 10 changed files with 46 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -205,7 +205,7 @@ public Object getIndex(DynamicObject matchData, int index, int length) {
public Object getIndexSymbol(DynamicObject matchData, DynamicObject index, NotProvided length) {
try {
final Rope value = Layouts.SYMBOL.getRope(index);
final int i = Layouts.REGEXP.getRegex(Layouts.MATCH_DATA.getRegexp(matchData)).nameToBackrefNumber(value.getBytes(), value.begin(), value.begin() + value.byteLength(), Layouts.MATCH_DATA.getRegion(matchData));
final int i = Layouts.REGEXP.getRegex(Layouts.MATCH_DATA.getRegexp(matchData)).nameToBackrefNumber(value.getBytes(), 0, value.byteLength(), Layouts.MATCH_DATA.getRegion(matchData));

return getIndex(matchData, i, NotProvided.INSTANCE);
} catch (final ValueException e) {
@@ -221,7 +221,7 @@ public Object getIndexSymbol(DynamicObject matchData, DynamicObject index, NotPr
public Object getIndexString(DynamicObject matchData, DynamicObject index, NotProvided length) {
try {
final Rope value = StringOperations.rope(index);
final int i = Layouts.REGEXP.getRegex(Layouts.MATCH_DATA.getRegexp(matchData)).nameToBackrefNumber(value.getBytes(), value.begin(), value.begin() + value.byteLength(), Layouts.MATCH_DATA.getRegion(matchData));
final int i = Layouts.REGEXP.getRegex(Layouts.MATCH_DATA.getRegexp(matchData)).nameToBackrefNumber(value.getBytes(), 0, value.byteLength(), Layouts.MATCH_DATA.getRegion(matchData));

return getIndex(matchData, i, NotProvided.INSTANCE);
}
Original file line number Diff line number Diff line change
@@ -86,10 +86,10 @@ public static Object matchCommon(RubyContext context, RopeNodes.MakeSubstringNod
final ByteList preprocessed = RegexpSupport.preprocess(context.getJRubyRuntime(), RopeOperations.getByteListReadOnly(regexpSourceRope), enc, new Encoding[] { null }, RegexpSupport.ErrorMode.RAISE);

final Regex r = new Regex(preprocessed.getUnsafeBytes(), preprocessed.getBegin(), preprocessed.getBegin() + preprocessed.getRealSize(), Layouts.REGEXP.getOptions(regexp).toJoniOptions(), checkEncoding(regexp, sourceRope, true));
final Matcher matcher = r.matcher(sourceRope.getBytes(), sourceRope.begin(), sourceRope.begin() + sourceRope.byteLength());
int range = sourceRope.begin() + sourceRope.byteLength();
final Matcher matcher = r.matcher(sourceRope.getBytes(), 0, sourceRope.byteLength());
int range = sourceRope.byteLength();

return matchCommon(context, makeSubstringNode, regexp, source, operator, setNamedCaptures, matcher, sourceRope.begin() + startPos, range);
return matchCommon(context, makeSubstringNode, regexp, source, operator, setNamedCaptures, matcher, startPos, range);
}

@TruffleBoundary
4 changes: 0 additions & 4 deletions truffle/src/main/java/org/jruby/truffle/core/rope/Rope.java
Original file line number Diff line number Diff line change
@@ -82,10 +82,6 @@ public final int depth() {
return ropeDepth;
}

public int begin() {
return 0;
}

@Override
public int hashCode() {
if (hashCode == 0) {
Original file line number Diff line number Diff line change
@@ -24,7 +24,6 @@
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.profiles.ConditionProfile;
import org.jcodings.Encoding;
import org.jcodings.specific.ASCIIEncoding;
import org.jcodings.specific.USASCIIEncoding;
@@ -122,7 +121,7 @@ public static String decodeUTF8(Rope rope) {
@TruffleBoundary
public static String decodeRope(Ruby runtime, Rope value) {
if (value instanceof LeafRope) {
int begin = value.begin();
int begin = 0;
int length = value.byteLength();

Encoding encoding = value.getEncoding();
@@ -230,7 +229,7 @@ public static void visitBytes(Rope rope, BytesVisitor visitor, int offset, int l
assert length <= rope.byteLength();

if (rope.getRawBytes() != null) {
visitor.accept(rope.getRawBytes(), rope.begin() + offset, length);
visitor.accept(rope.getRawBytes(), offset, length);
} else if (rope instanceof ConcatRope) {
final ConcatRope concat = (ConcatRope) rope;

@@ -586,9 +585,9 @@ public static int cmp(Rope string, Rope other) {
// [slightly] understand it), in some cases, when two (or more?)
// arrays are being accessed, the member reference is actually
// faster. this is one of those cases...
for ( ; ++offset < len && bytes[string.begin() + offset] == otherBytes[other.begin() + offset]; ) ;
for ( ; ++offset < len && bytes[offset] == otherBytes[offset]; ) ;
if (offset < len) {
return (bytes[string.begin() + offset]&0xFF) > (otherBytes[other.begin() + offset]&0xFF) ? 1 : -1;
return (bytes[offset]&0xFF) > (otherBytes[offset]&0xFF) ? 1 : -1;
}
return size == other.byteLength() ? 0 : size == len ? -1 : 1;
}
Original file line number Diff line number Diff line change
@@ -211,11 +211,11 @@ public boolean fnmatch(DynamicObject pattern, DynamicObject path, int flags) {
final Rope pathRope = rope(path);

return Dir.fnmatch(patternRope.getBytes(),
patternRope.begin(),
patternRope.begin() + patternRope.byteLength(),
0,
patternRope.byteLength(),
pathRope.getBytes(),
pathRope.begin(),
pathRope.begin() + pathRope.byteLength(),
0,
pathRope.byteLength(),
flags) != Dir.FNM_NOMATCH;
}

@@ -408,12 +408,12 @@ public int write(DynamicObject file, DynamicObject string) {
final Rope rope = rope(string);

if (getContext().getDebugStandardOut() != null && fd == STDOUT) {
getContext().getDebugStandardOut().write(rope.getBytes(), rope.begin(), rope.byteLength());
getContext().getDebugStandardOut().write(rope.getBytes(), 0, rope.byteLength());
return rope.byteLength();
}

// TODO (eregon, 11 May 2015): review consistency under concurrent modification
final ByteBuffer buffer = ByteBuffer.wrap(rope.getBytes(), rope.begin(), rope.byteLength());
final ByteBuffer buffer = ByteBuffer.wrap(rope.getBytes(), 0, rope.byteLength());

int total = 0;

Original file line number Diff line number Diff line change
@@ -145,15 +145,15 @@ public Object searchRegion(DynamicObject regexp, DynamicObject string, int start
final Encoding enc = RegexpNodes.checkEncoding(regexp, stringRope, true);
ByteList preprocessed = RegexpSupport.preprocess(getContext().getJRubyRuntime(), RopeOperations.getByteListReadOnly(regexpSourceRope), enc, new Encoding[]{null}, RegexpSupport.ErrorMode.RAISE);
Rope preprocessedRope = RegexpNodes.shimModifiers(StringOperations.ropeFromByteList(preprocessed));
final Regex r = new Regex(preprocessedRope.getBytes(), preprocessedRope.begin(), preprocessedRope.begin() + preprocessedRope.byteLength(), Layouts.REGEXP.getRegex(regexp).getOptions(), RegexpNodes.checkEncoding(regexp, stringRope, true));
final Matcher matcher = r.matcher(stringRope.getBytes(), stringRope.begin(), stringRope.begin() + stringRope.byteLength());
final Regex r = new Regex(preprocessedRope.getBytes(), 0, preprocessedRope.byteLength(), Layouts.REGEXP.getRegex(regexp).getOptions(), RegexpNodes.checkEncoding(regexp, stringRope, true));
final Matcher matcher = r.matcher(stringRope.getBytes(), 0, stringRope.byteLength());

if (forward) {
// Search forward through the string.
return RegexpNodes.matchCommon(getContext(), makeSubstringNode, regexp, string, false, false, matcher, start + stringRope.begin(), end + stringRope.begin());
return RegexpNodes.matchCommon(getContext(), makeSubstringNode, regexp, string, false, false, matcher, start, end);
} else {
// Search backward through the string.
return RegexpNodes.matchCommon(getContext(), makeSubstringNode, regexp, string, false, false, matcher, end + stringRope.begin(), start + stringRope.begin());
return RegexpNodes.matchCommon(getContext(), makeSubstringNode, regexp, string, false, false, matcher, end, start);
}
}

Original file line number Diff line number Diff line change
@@ -87,7 +87,6 @@
import org.jruby.truffle.core.encoding.EncodingNodes;
import org.jruby.truffle.core.encoding.EncodingOperations;
import org.jruby.truffle.core.rope.CodeRange;
import org.jruby.truffle.core.rope.RepeatingRope;
import org.jruby.truffle.core.rope.Rope;
import org.jruby.truffle.core.rope.RopeBuffer;
import org.jruby.truffle.core.rope.RopeConstants;
@@ -219,7 +218,7 @@ public DynamicObject stringAwkSplit(DynamicObject string, int lim) {
int i = lim > 0 ? 1 : 0;

byte[]bytes = rope.getBytes();
int p = rope.begin();
int p = 0;
int ptr = p;
int len = rope.byteLength();
int end = p + len;
@@ -491,8 +490,8 @@ public int stringCompareSubstring(VirtualFrame frame, DynamicObject string, Dyna
final Rope otherRope = StringOperations.rope(other);

// TODO (nirvdrum 21-Jan-16): Reimplement with something more friendly to rope byte[] layout?
return ByteList.memcmp(rope.getBytes(), rope.begin(), size,
otherRope.getBytes(), otherRope.begin() + start, size);
return ByteList.memcmp(rope.getBytes(), 0, size,
otherRope.getBytes(), start, size);
}

}
@@ -678,7 +677,7 @@ public Object stringFindCharacter(DynamicObject string, int offset,
}

final Encoding enc = rope.getEncoding();
final int clen = StringSupport.preciseLength(enc, rope.getBytes(), rope.begin(), rope.begin() + rope.byteLength());
final int clen = StringSupport.preciseLength(enc, rope.getBytes(), 0, rope.byteLength());

final DynamicObject ret;
if (StringSupport.MBCLEN_CHARFOUND_P(clen)) {
@@ -850,7 +849,7 @@ private int index(Rope source, Rope other, int offset, Encoding enc) {

if (sourceLen - offset < otherLen) return -1;
byte[]bytes = source.getBytes();
int p = source.begin();
int p = 0;
int end = p + source.byteLength();
if (offset != 0) {
offset = source.isSingleByteOptimizable() ? offset : StringSupport.offset(enc, bytes, p, end, offset);
@@ -859,9 +858,9 @@ private int index(Rope source, Rope other, int offset, Encoding enc) {
if (otherLen == 0) return offset;

while (true) {
int pos = indexOf(source, other, p - source.begin());
int pos = indexOf(source, other, p);
if (pos < 0) return pos;
pos -= (p - source.begin());
pos -= p;
int t = enc.rightAdjustCharHead(bytes, p, p + pos, end);
if (t == p + pos) return pos + offset;
if ((sourceLen -= t - p) <= 0) return -1;
@@ -875,10 +874,10 @@ private int indexOf(Rope sourceRope, Rope otherRope, int fromIndex) {
// Taken from org.jruby.util.ByteList.indexOf.

final byte[] source = sourceRope.getBytes();
final int sourceOffset = sourceRope.begin();
final int sourceOffset = 0;
final int sourceCount = sourceRope.byteLength();
final byte[] target = otherRope.getBytes();
final int targetOffset = otherRope.begin();
final int targetOffset = 0;
final int targetCount = otherRope.byteLength();

if (fromIndex >= sourceCount) return (targetCount == 0 ? sourceCount : -1);
@@ -997,9 +996,9 @@ public Object stringCharacterIndex(DynamicObject string, DynamicObject pattern,
final Rope patternRope = rope(pattern);

final int total = stringRope.byteLength();
int p = stringRope.begin();
int p = 0;
final int e = p + total;
int pp = patternRope.begin();
int pp = 0;
final int pe = pp + patternRope.byteLength();
int s;
int ss;
@@ -1108,7 +1107,7 @@ public Object stringByteIndex(DynamicObject string, int index, int start,

final Rope rope = rope(string);
final Encoding enc = rope.getEncoding();
int p = rope.begin();
int p = 0;
final int e = p + rope.byteLength();

int i, k = index;
@@ -1132,7 +1131,7 @@ public Object stringByteIndex(DynamicObject string, int index, int start,
if (indexTooLargeProfile.profile(i < k)) {
return nil();
} else {
return p - rope.begin();
return p;
}
}

@@ -1157,9 +1156,9 @@ public Object stringByteIndex(DynamicObject string, DynamicObject pattern, int o
}

final Encoding encoding = StringOperations.checkEncoding(getContext(), string, pattern, this);
int p = stringRope.begin();
int p = 0;
final int e = p + stringRope.byteLength();
int pp = patternRope.begin();
int pp = 0;
final int pe = pp + patternRope.byteLength();
int s;
int ss;
@@ -1182,7 +1181,7 @@ public Object stringByteIndex(DynamicObject string, DynamicObject pattern, int o
final int c = StringSupport.preciseLength(encoding, stringBytes, s, e);

if (StringSupport.MBCLEN_CHARFOUND_P(c)) {
return s - stringRope.begin();
return s;
} else {
return nil();
}
@@ -1242,7 +1241,7 @@ public int stringPreviousByteIndexFixedWidthEncoding(DynamicObject string, int i
@TruffleBoundary
public Object stringPreviousByteIndex(DynamicObject string, int index) {
final Rope rope = rope(string);
final int p = rope.begin();
final int p = 0;
final int end = p + rope.byteLength();

final int b = rope.getEncoding().prevCharHead(rope.getBytes(), p, p + index, end);
@@ -1285,7 +1284,7 @@ public DynamicObject stringCopyFrom(DynamicObject string, DynamicObject other, i
if(negativeDestinationOffsetProfile.profile(dst < 0)) dst = 0;
if(sizeTooLargeInStringProfile.profile(cnt > sz - dst)) cnt = sz - dst;

System.arraycopy(otherRope.getBytes(), otherRope.begin() + src, stringBytes.getUnsafeBytes(), stringBytes.begin() + dest, cnt);
System.arraycopy(otherRope.getBytes(), src, stringBytes.getUnsafeBytes(), stringBytes.begin() + dest, cnt);

StringOperations.setRope(string, StringOperations.ropeFromByteList(stringBytes));

@@ -1312,7 +1311,7 @@ protected boolean offsetTooLargeRaw(int offset, DynamicObject string) {

// TODO (nirvdrum 21-Jan-16) Verify whether we still need this method as we never have spare capacity allocated with ropes.
final Rope rope = rope(string);
return offset >= (rope.byteLength() - rope.begin());
return offset >= rope.byteLength();
}

}
@@ -1429,7 +1428,7 @@ public DynamicObject stringPattern(DynamicObject stringClass, int size, DynamicO
// TODO (nirvdrum 21-Jan-16): Investigate whether using a ConcatRope (potentially combined with a RepeatingRope) would be better here.
if (! rope.isEmpty()) {
for (int n = 0; n < size; n += rope.byteLength()) {
System.arraycopy(rope.getBytes(), rope.begin(), bytes, n, Math.min(rope.byteLength(), size - n));
System.arraycopy(rope.getBytes(), 0, bytes, n, Math.min(rope.byteLength(), size - n));
}
}

Original file line number Diff line number Diff line change
@@ -42,7 +42,6 @@
import com.oracle.truffle.api.source.SourceSection;
import org.jcodings.Encoding;
import org.jcodings.specific.ASCIIEncoding;
import org.jcodings.specific.USASCIIEncoding;
import org.jcodings.specific.UTF8Encoding;
import org.jruby.truffle.RubyContext;
import org.jruby.truffle.core.CoreClass;
@@ -77,7 +76,6 @@
import org.jruby.truffle.core.rope.RopeNodes.MakeRepeatingNode;
import org.jruby.truffle.core.rope.RopeNodesFactory;
import org.jruby.truffle.core.rope.RopeOperations;
import org.jruby.truffle.core.rope.SubstringRope;
import org.jruby.truffle.core.rubinius.StringPrimitiveNodes;
import org.jruby.truffle.core.rubinius.StringPrimitiveNodesFactory;
import org.jruby.truffle.language.NotProvided;
@@ -102,7 +100,6 @@
import java.util.Arrays;

import static org.jruby.truffle.core.rope.RopeConstants.EMPTY_ASCII_8BIT_ROPE;
import static org.jruby.truffle.core.rope.RopeConstants.EMPTY_UTF8_ROPE;
import static org.jruby.truffle.core.string.StringOperations.encoding;
import static org.jruby.truffle.core.string.StringOperations.rope;

@@ -1381,7 +1378,7 @@ public Object lstripBangSingleByte(DynamicObject string) {
// Taken from org.jruby.RubyString#lstrip_bang19 and org.jruby.RubyString#singleByteLStrip.

final Rope rope = rope(string);
final int s = rope.begin();
final int s = 0;
final int end = s + rope.byteLength();
final byte[] bytes = rope.getBytes();

@@ -1403,7 +1400,7 @@ public Object lstripBang(DynamicObject string) {

final Rope rope = rope(string);
final Encoding enc = RopeOperations.STR_ENC_GET(rope);
final int s = rope.begin();
final int s = 0;
final int end = s + rope.byteLength();
final byte[] bytes = rope.getBytes();

@@ -1499,7 +1496,7 @@ public int ord(DynamicObject string) {
final Rope rope = rope(string);

try {
return codePoint(rope.getEncoding(), rope.getBytes(), rope.begin(), rope.begin() + rope.byteLength());
return codePoint(rope.getEncoding(), rope.getBytes(), 0, rope.byteLength());
} catch (IllegalArgumentException e) {
CompilerDirectives.transferToInterpreter();
throw new RaiseException(coreExceptions().argumentError(e.getMessage(), this));
@@ -1646,7 +1643,7 @@ public DynamicObject swapcaseSingleByte(DynamicObject string,
return nil();
}

final int s = rope.begin();
final int s = 0;
final int end = s + rope.byteLength();
final byte[] bytes = rope.getBytesCopy();

@@ -1976,7 +1973,7 @@ public Object sum(VirtualFrame frame, DynamicObject string, long bits) {

final Rope rope = rope(string);
final byte[] bytes = rope.getBytes();
int p = rope.begin();
int p = 0;
final int len = rope.byteLength();
final int end = p + len;

Original file line number Diff line number Diff line change
@@ -117,7 +117,7 @@ public DynamicObject getSymbol(Rope rope) {

final DynamicObject symbolClass = context.getCoreLibrary().getSymbolClass();
final Rope flattenedRope = RopeOperations.flatten(rope);
final String string = ByteList.decode(flattenedRope.getBytes(), flattenedRope.begin(), flattenedRope.byteLength(), "ISO-8859-1");
final String string = ByteList.decode(flattenedRope.getBytes(), 0, flattenedRope.byteLength(), "ISO-8859-1");

final DynamicObject newSymbol = Layouts.SYMBOL.createSymbol(
Layouts.CLASS.getInstanceFactory(symbolClass),
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@
import org.jruby.truffle.core.rope.Rope;
import org.jruby.truffle.core.rope.RopeOperations;
import org.jruby.truffle.core.string.StringOperations;
import org.jruby.util.ByteList;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
@@ -236,7 +235,7 @@ public BubbleBabbleNode(RubyContext context, SourceSection sourceSection) {
@Specialization(guards = "isRubyString(message)")
public DynamicObject bubblebabble(DynamicObject message) {
final Rope rope = StringOperations.rope(message);
return createString(BubbleBabble.bubblebabble(rope.getBytes(), rope.begin(), rope.byteLength()));
return createString(BubbleBabble.bubblebabble(rope.getBytes(), 0, rope.byteLength()));
}

}

0 comments on commit 7f8d612

Please sign in to comment.