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

Commits on Jun 29, 2018

  1. handle setEncoding better for shared strings 

    only set when changed - no need to modify() (copy) otherwise
+ do not setEncoding for new strings when can use constructor
    @kares
    kares committed Jun 29, 2018
    Copy the full SHA
    81dfd2d View commit details

  2. [fix] byte-list cached version of readline 

    which never worked as intended, the slow-path is taken when
specifying opts e.g. `file.each_line(limit)`

performance is ~ same -- but there's less byte[] allocation

+ reviewed - cleaned up warnings in OpenFile
    @kares
    kares committed Jun 29, 2018
    Copy the full SHA
    deead6d View commit details

  3. [refactor] drop byte-list cache on IO.readline and friends 

    still needs tricks to handle string sharing - can not get this
reasonably right with current setup.
    @kares
    kares committed Jun 29, 2018
    Copy the full SHA
    4c991f7 View commit details

  4. [refactor] do not check Hash options twice

    @kares
    kares committed Jun 29, 2018
    Copy the full SHA
    86736e3 View commit details
11 changes: 4 additions & 7 deletions core/src/main/java/org/jruby/RubyFixnum.java
View file
Original file line number Diff line number Diff line change
@@ -399,10 +399,8 @@ public RubyString to_s(IRubyObject[] args) {

@Override
public RubyString to_s() {
ByteList bl = ConvertBytes.longToByteList(value, 10);
RubyString str = getRuntime().newString(bl);
str.setEncoding(USASCIIEncoding.INSTANCE);
return str;
ByteList bytes = ConvertBytes.longToByteList(value, 10);
return RubyString.newString(getRuntime(), bytes, USASCIIEncoding.INSTANCE);
}

@Override
@@ -411,9 +409,8 @@ public RubyString to_s(IRubyObject arg0) {
if (base < 2 || base > 36) {
throw getRuntime().newArgumentError("illegal radix " + base);
}
ByteList bl = ConvertBytes.longToByteList(value, base);
bl.setEncoding(USASCIIEncoding.INSTANCE);
return getRuntime().newString(bl);
ByteList bytes = ConvertBytes.longToByteList(value, base);
return RubyString.newString(getRuntime(), bytes, USASCIIEncoding.INSTANCE);
}

/** fix_to_sym
188 changes: 73 additions & 115 deletions core/src/main/java/org/jruby/RubyIO.java
View file
Original file line number Diff line number Diff line change
@@ -679,27 +679,22 @@ public IRubyObject reopen(ThreadContext context, IRubyObject[] args) {
}

public IRubyObject getline(ThreadContext context, IRubyObject separator) {
return getlineImpl(context, separator, -1, false, null);
return getlineImpl(context, separator, -1, false);
}

/**
* getline using logic of gets. If limit is -1 then read unlimited amount.
*
*/
public IRubyObject getline(ThreadContext context, IRubyObject separator, long limit) {
return getlineImpl(context, separator, (int) limit, false, null);
}

private IRubyObject getline(ThreadContext context, IRubyObject separator, long limit, ByteListCache cache) {
return getlineImpl(context, separator, (int) limit, false, cache);
return getlineImpl(context, separator, (int) limit, false);
}

/**
* getline using logic of gets. If limit is -1 then read unlimited amount.
* mri: rb_io_getline_1 (mostly)
*/
private IRubyObject getlineImpl(ThreadContext context, IRubyObject rs,
final int limit, final boolean chomp, final ByteListCache cache) {
private IRubyObject getlineImpl(ThreadContext context, IRubyObject rs, final int limit, final boolean chomp) {
Ruby runtime = context.runtime;

final OpenFile fptr = getOpenFileChecked();
@@ -726,15 +721,15 @@ private IRubyObject getlineImpl(ThreadContext context, IRubyObject rs,
return fptr.getlineFast(context, enc, this, chomp);
}

return getlineImplSlowPart(context, fptr, str, rs, limit, chomp, cache);
return getlineImplSlowPart(context, fptr, str, rs, limit, chomp);

} finally {
if (locked) fptr.unlock();
}
}

private IRubyObject getlineImplSlowPart(ThreadContext context, final OpenFile fptr,
RubyString str, IRubyObject rs, final int limit, final boolean chomp, final ByteListCache cache) {
RubyString str, IRubyObject rs, final int limit, final boolean chomp) {

Ruby runtime = context.runtime;

@@ -750,7 +745,7 @@ private IRubyObject getlineImplSlowPart(ThreadContext context, final OpenFile fp
boolean chompCR = chomp;

fptr.SET_BINARY_MODE();
Encoding enc = getReadEncoding();
final Encoding enc = getReadEncoding();

if (rs != context.nil) {
RubyString rsStr = (RubyString) rs;
@@ -780,69 +775,64 @@ private IRubyObject getlineImplSlowPart(ThreadContext context, final OpenFile fp
chompCR = chomp && rslen == 1 && newline == '\n';
}

try {
ByteList[] strPtr = { str != null ? str.getByteList() : null };

int[] limit_p = {limit};
while ((c = fptr.appendline(context, newline, strPtr, limit_p)) != OpenFile.EOF) {
int s, p, pp, e;

byte[] strBytes = strPtr[0].getUnsafeBytes();
int realSize = strPtr[0].getRealSize();
int begin = strPtr[0].getBegin();

if (c == newline) {
if (realSize < rslen) continue;
s = begin;
e = s + realSize;
p = e - rslen;
pp = enc.leftAdjustCharHead(strBytes, s, p, e);
if (pp != p) continue;
if (ByteList.memcmp(strBytes, p, rsptrBytes, rsptr, rslen) == 0) {
if (chomp) {
if (chompCR && p > s && strBytes[p-1] == '\r') --p;
strPtr[0].length(p - s);
}
break;
}
}
if (limit_p[0] == 0) {
s = begin;
p = s + realSize;
pp = enc.leftAdjustCharHead(strBytes, s, p - 1, p);
if (extraLimit != 0 &&
StringSupport.MBCLEN_NEEDMORE_P(StringSupport.preciseLength(enc, strBytes, pp, p))) {
limit_p[0] = 1;
extraLimit--;
} else {
noLimit = true;
break;
final ByteList[] strPtr = { str != null ? str.getByteList() : null };
final int[] limit_p = { limit };

while ((c = fptr.appendline(context, newline, strPtr, limit_p)) != OpenFile.EOF) {
int s, p, pp, e;

final byte[] strBytes = strPtr[0].getUnsafeBytes();
final int realSize = strPtr[0].getRealSize();
final int begin = strPtr[0].getBegin();

if (c == newline) {
if (realSize < rslen) continue;
s = begin;
e = s + realSize;
p = e - rslen;
pp = enc.leftAdjustCharHead(strBytes, s, p, e);
if (pp != p) continue;
if (ByteList.memcmp(strBytes, p, rsptrBytes, rsptr, rslen) == 0) {
if (chomp) {
if (chompCR && p > s && strBytes[p-1] == '\r') --p;
strPtr[0].length(p - s);
}
break;
}
}
// limit = limit_p[0];
if (strPtr[0] != null) {
if (str != null) {
str.setValue(strPtr[0]);
if (limit_p[0] == 0) {
s = begin;
p = s + realSize;
pp = enc.leftAdjustCharHead(strBytes, s, p - 1, p);
if (extraLimit != 0 &&
StringSupport.MBCLEN_NEEDMORE_P(StringSupport.preciseLength(enc, strBytes, pp, p))) {
limit_p[0] = 1;
extraLimit--;
} else {
str = runtime.newString(strPtr[0]);
noLimit = true;
break;
}
}

if (rspara && c != OpenFile.EOF) {
// FIXME: This may block more often than it should, to clean up extraneous newlines
fptr.swallow(context, '\n');
}
}
// limit = limit_p[0];
if (strPtr[0] != null) {
if (str != null) {
EncodingUtils.ioEncStr(runtime, str, fptr);
str.setValue(strPtr[0]);
} else {
str = runtime.newString(strPtr[0]);
}
} finally {
//
}

if (str != null && !noLimit) {
fptr.incrementLineno(runtime, this);
if (rspara && c != OpenFile.EOF) {
// FIXME: This may block more often than it should, to clean up extraneous newlines
fptr.swallow(context, '\n');
}
if (str != null) { // io_enc_str :
str.setTaint(true);
str.setEncoding(enc);
}

if (str != null && !noLimit) fptr.incrementLineno(runtime, this);

return str == null ? context.nil : str;
}
@@ -867,15 +857,6 @@ public Encoding getInputEncoding() {
return openFile.inputEncoding(getRuntime());
}

private static final String VENDOR;
static { String v = SafePropertyAccessor.getProperty("java.VENDOR") ; VENDOR = (v == null) ? "" : v; };
private static final String msgEINTR = "Interrupted system call";

// FIXME: We needed to use this to raise an appropriate error somewhere...find where...I think IRB related when suspending process?
public static boolean restartSystemCall(Exception e) {
return VENDOR.startsWith("Apple") && e.getMessage().equals(msgEINTR);
}

// IO class methods.

@JRubyMethod(name = "new", rest = true, meta = true)
@@ -2354,7 +2335,7 @@ public IRubyObject gets(ThreadContext context, IRubyObject rs, IRubyObject limit
private static final Getline.Callback<RubyIO, IRubyObject> GETLINE = new Getline.Callback<RubyIO, IRubyObject>() {
@Override
public IRubyObject getline(ThreadContext context, RubyIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
IRubyObject result = self.getlineImpl(context, rs, limit, chomp, null);
IRubyObject result = self.getlineImpl(context, rs, limit, chomp);

if (result != context.nil) context.setLastLine(result);

@@ -2365,10 +2346,9 @@ public IRubyObject getline(ThreadContext context, RubyIO self, IRubyObject rs, i
private static final Getline.Callback<RubyIO, RubyIO> GETLINE_YIELD = new Getline.Callback<RubyIO, RubyIO>() {
@Override
public RubyIO getline(ThreadContext context, RubyIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
ByteListCache cache = new ByteListCache();

IRubyObject line;
while ((line = self.getlineImpl(context, rs, limit, chomp, cache)) != context.nil) {
while ((line = self.getlineImpl(context, rs, limit, chomp)) != context.nil) {
block.yieldSpecific(context, line);
}

@@ -2379,11 +2359,11 @@ public RubyIO getline(ThreadContext context, RubyIO self, IRubyObject rs, int li
private static final Getline.Callback<RubyIO, RubyArray> GETLINE_ARY = new Getline.Callback<RubyIO, RubyArray>() {
@Override
public RubyArray getline(ThreadContext context, RubyIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
ByteListCache cache = new ByteListCache();

RubyArray ary = context.runtime.newArray();
IRubyObject line;

while ((line = self.getlineImpl(context, rs, limit, chomp, cache)) != context.nil) {
while ((line = self.getlineImpl(context, rs, limit, chomp)) != context.nil) {
ary.append(line);
}

@@ -4503,71 +4483,47 @@ public void removeBlockingThread(RubyThread thread) {
if (fptr != null) fptr.removeBlockingThread(thread);
}

/**
* Caching reference to allocated byte-lists, allowing for internal byte[] to be
* reused, rather than reallocated.
*
* Predominately used on {@link RubyIO#getline(Ruby, ByteList)} and variants.
*
* @author realjenius
*/
private static class ByteListCache {

private byte[] buffer = ByteList.NULL_ARRAY;

final void release(ByteList bytes) {
buffer = bytes.getUnsafeBytes();
}

final ByteList allocate() {
return new ByteList(buffer, 0, buffer.length, false);
}
}

/**
* See http://ruby-doc.org/core-1.9.3/IO.html#method-c-new for the format of modes in options
*/
protected IOOptions updateIOOptionsFromOptions(ThreadContext context, RubyHash options, IOOptions ioOptions) {
if (options == null || options.isNil()) return ioOptions;
if (options == null || options == context.nil) return ioOptions;

final Ruby runtime = context.runtime;

final RubySymbol mode = runtime.newSymbol("mode");
if (options.containsKey(mode)) {
ioOptions = parseIOOptions(options.fastARef(mode));
final IRubyObject mode = options.fastARef(runtime.newSymbol("mode"));
if (mode != null) {
ioOptions = parseIOOptions(mode);
}

// This duplicates the non-error behavior of MRI 1.9: the
// :binmode option is ORed in with other options. It does
// not obliterate what came before.

final RubySymbol binmode = runtime.newSymbol("binmode");
if (options.containsKey(binmode) && options.fastARef(binmode).isTrue()) {
if (isTrue(options.fastARef(binmode))) {
ioOptions = newIOOptions(runtime, ioOptions, ModeFlags.BINARY);
}

// This duplicates the non-error behavior of MRI 1.9: the
// :binmode option is ORed in with other options. It does
// not obliterate what came before.

if (options.containsKey(binmode) && options.fastARef(binmode).isTrue()) {
if (isTrue(options.fastARef(binmode))) {
ioOptions = newIOOptions(runtime, ioOptions, ModeFlags.BINARY);
}

final RubySymbol textmode = runtime.newSymbol("textmode");
if (options.containsKey(textmode) && options.fastARef(textmode).isTrue()) {
if (isTrue(options.fastARef(runtime.newSymbol("textmode")))) {
ioOptions = newIOOptions(runtime, ioOptions, ModeFlags.TEXT);
}

final RubySymbol open_args = runtime.newSymbol("open_args");
// TODO: Waaaay different than MRI. They uniformly have all opening logic
// do a scan of args before anything opens. We do this logic in a less
// consistent way. We should consider re-impling all IO/File construction
// logic.
if (options.containsKey(open_args)) {
IRubyObject args = options.fastARef(open_args);

RubyArray openArgs = args.convertToArray();
IRubyObject open_args = options.fastARef(runtime.newSymbol("open_args"));
if (open_args != null) {
RubyArray openArgs = open_args.convertToArray();

for (int i = 0; i < openArgs.size(); i++) {
IRubyObject arg = openArgs.eltInternal(i);
@@ -4587,6 +4543,8 @@ protected IOOptions updateIOOptionsFromOptions(ThreadContext context, RubyHash o
return ioOptions;
}

private static boolean isTrue(IRubyObject val) { return val != null && val.isTrue(); }

static final Set<String> ALL_SPAWN_OPTIONS;
static final String[] UNSUPPORTED_SPAWN_OPTIONS;

@@ -4878,22 +4836,22 @@ private static IOSites sites(ThreadContext context) {

@Deprecated
public IRubyObject getline(Ruby runtime, ByteList separator) {
return getline(runtime.getCurrentContext(), runtime.newString(separator), -1, null);
return getline(runtime.getCurrentContext(), runtime.newString(separator), -1);
}

@Deprecated
public IRubyObject getline(Ruby runtime, ByteList separator, long limit) {
return getline(runtime.getCurrentContext(), runtime.newString(separator), limit, null);
return getline(runtime.getCurrentContext(), runtime.newString(separator), limit);
}

@Deprecated
public IRubyObject getline(ThreadContext context, ByteList separator) {
return getline(context, RubyString.newString(context.runtime, separator), -1, null);
return getline(context, RubyString.newString(context.runtime, separator), -1);
}

@Deprecated
public IRubyObject getline(ThreadContext context, ByteList separator, long limit) {
return getline(context, RubyString.newString(context.runtime, separator), limit, null);
return getline(context, RubyString.newString(context.runtime, separator), limit);
}

@Deprecated
Loading