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

Commits on Jun 28, 2016

  1. Copy the full SHA
    3fe3a18 View commit details
  2. Add new mechanism for visiting all Hash elements without alloc.

    This new mechanism passes in state that makes most Visitor
    construction unnecessary: thread context, currently-visiting Hash,
    and a generic state object.
    headius committed Jun 28, 2016
    Copy the full SHA
    7da1d22 View commit details

Commits on Jun 29, 2016

  1. Use new visitor logic everywhere.

    Nearly all of these are now allocated once, like a lambda. A few
    require more state to be passed in than is reasonable for a
    generic visitor, or require state to be calculated during visit
    and retrieved at the end. These cases were at least reduced to
    only allocate a visitor, rather than a visitor and a state-holding
    object.
    headius committed Jun 29, 2016
    Copy the full SHA
    021c19b View commit details
  2. Deprecate old visitAll to prevent mix-ups.

    See MapJavaProxy's RubyHashMap subclass; it previously override
    the old visitAll, which broke Map instances returned from Java
    because that visitAll was not being used. This will help ensure
    no subclasses are overriding the wrong method. I don't expect
    there's many (or maybe any) other cases in the wild.
    headius committed Jun 29, 2016
    Copy the full SHA
    468b4e3 View commit details
20 changes: 7 additions & 13 deletions core/src/main/java/org/jruby/RubyArray.java
Original file line number Diff line number Diff line change
@@ -208,6 +208,10 @@ public static RubyArray newArrayLight(Ruby runtime, IRubyObject obj) {
return new RubyArrayOneObject(runtime, obj);
}

public static RubyArray newArrayLight(Ruby runtime, IRubyObject car, IRubyObject cdr) {
return new RubyArrayTwoObject(runtime, car, cdr);
}

public static RubyArray newArrayLight(Ruby runtime, IRubyObject... objs) {
return new RubyArray(runtime, objs, false);
}
@@ -3180,12 +3184,7 @@ public IRubyObject uniq_bang19(ThreadContext context, Block block) {

realLength = 0;

hash.visitAll(new RubyHash.Visitor() {
@Override
public void visit(IRubyObject key, IRubyObject value) {
append(value);
}
});
hash.visitAll(context, RubyHash.AppendValueVisitor, this);
return this;
}

@@ -3216,13 +3215,8 @@ public IRubyObject uniq19(ThreadContext context, Block block) {
if (!block.isGiven()) return uniq(context);
RubyHash hash = makeHash(context, block);

final RubyArray result = new RubyArray(context.runtime, getMetaClass(), hash.size());
hash.visitAll(new RubyHash.Visitor() {
@Override
public void visit(IRubyObject key, IRubyObject value) {
result.append(value);
}
});
RubyArray result = new RubyArray(context.runtime, getMetaClass(), hash.size());
hash.visitAll(context, RubyHash.AppendValueVisitor, result);
return result;
}

518 changes: 295 additions & 223 deletions core/src/main/java/org/jruby/RubyHash.java

Large diffs are not rendered by default.

30 changes: 16 additions & 14 deletions core/src/main/java/org/jruby/RubyThread.java
Original file line number Diff line number Diff line change
@@ -712,20 +712,7 @@ public static IRubyObject handle_interrupt(ThreadContext context, IRubyObject se

final RubyHash mask = (RubyHash) TypeConverter.convertToType(_mask, context.runtime.getHash(), "to_hash");

mask.visitAll(new RubyHash.Visitor() {
@Override
public void visit(IRubyObject key, IRubyObject value) {
if (value instanceof RubySymbol) {
RubySymbol sym = (RubySymbol) value;
switch (sym.toString()) {
case "immediate" : return;
case "on_blocking" : return;
case "never" : return;
default : throw key.getRuntime().newArgumentError("unknown mask signature");
}
}
}
});
mask.visitAll(context, HandleInterruptVisitor, null);

RubyThread th = context.getThread();
th.interruptMaskStack.add(mask);
@@ -744,6 +731,21 @@ public void visit(IRubyObject key, IRubyObject value) {
}
}

private static final RubyHash.VisitorWithState HandleInterruptVisitor = new RubyHash.VisitorWithState() {
@Override
public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Object state) {
if (value instanceof RubySymbol) {
RubySymbol sym = (RubySymbol) value;
switch (sym.toString()) {
case "immediate" : return;
case "on_blocking" : return;
case "never" : return;
default : throw key.getRuntime().newArgumentError("unknown mask signature");
}
}
}
};

@JRubyMethod(name = "pending_interrupt?", meta = true, optional = 1)
public static IRubyObject pending_interrupt_p(ThreadContext context, IRubyObject self, IRubyObject[] args) {
return context.getThread().pending_interrupt_p(context, args);
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ public static CheckArityInstr decode(IRReaderDecoder d) {
}

public void checkArity(ThreadContext context, StaticScope scope, Object[] args, Block.Type blockType) {
IRRuntimeHelpers.checkArity(context.runtime, scope, args, required, opt, rest, receivesKeywords, restKey, blockType);
IRRuntimeHelpers.checkArity(context, scope, args, required, opt, rest, receivesKeywords, restKey, blockType);
}

@Override
34 changes: 18 additions & 16 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Original file line number Diff line number Diff line change
@@ -535,12 +535,12 @@ public static Block getBlockFromObject(ThreadContext context, Object value) {
return block;
}

public static void checkArity(Ruby runtime, StaticScope scope, Object[] args, int required, int opt, boolean rest,
public static void checkArity(ThreadContext context, StaticScope scope, Object[] args, int required, int opt, boolean rest,
boolean receivesKwargs, int restKey, Block.Type blockType) {
int argsLength = args.length;
RubyHash keywordArgs = extractKwargsHash(args, required, receivesKwargs);

if (restKey == -1 && keywordArgs != null) checkForExtraUnwantedKeywordArgs(runtime, scope, keywordArgs);
if (restKey == -1 && keywordArgs != null) checkForExtraUnwantedKeywordArgs(context, scope, keywordArgs);

// keyword arguments value is not used for arity checking.
if (keywordArgs != null) argsLength -= 1;
@@ -549,7 +549,7 @@ public static void checkArity(Ruby runtime, StaticScope scope, Object[] args, in
// System.out.println("NUMARGS: " + argsLength + ", REQUIRED: " + required + ", OPT: " + opt + ", AL: " + args.length + ",RKW: " + receivesKwargs );
// System.out.println("ARGS[0]: " + args[0]);

Arity.raiseArgumentError(runtime, argsLength, required, required + opt);
Arity.raiseArgumentError(context.runtime, argsLength, required, required + opt);
}
}

@@ -588,21 +588,23 @@ public static RubyHash extractKwargsHash(Object[] args, int requiredArgsCount, b
return null;
}

public static void checkForExtraUnwantedKeywordArgs(final Ruby runtime, final StaticScope scope, RubyHash keywordArgs) {
keywordArgs.visitAll(new RubyHash.Visitor() {
@Override
public void visit(IRubyObject key, IRubyObject value) {
String keyAsString = key.asJavaString();
int slot = scope.isDefined((keyAsString));

// Found name in higher variable scope. Therefore non for this block/method def.
if ((slot >> 16) > 0) throw runtime.newArgumentError("unknown keyword: " + keyAsString);
// Could not find it anywhere.
if (((short) (slot & 0xffff)) < 0) throw runtime.newArgumentError("unknown keyword: " + keyAsString);
}
});
public static void checkForExtraUnwantedKeywordArgs(ThreadContext context, final StaticScope scope, RubyHash keywordArgs) {
keywordArgs.visitAll(context, CheckUnwantedKeywordsVisitor, scope);
}

private static final RubyHash.VisitorWithState<StaticScope> CheckUnwantedKeywordsVisitor = new RubyHash.VisitorWithState<StaticScope>() {
@Override
public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, StaticScope scope) {
String keyAsString = key.asJavaString();
int slot = scope.isDefined((keyAsString));

// Found name in higher variable scope. Therefore non for this block/method def.
if ((slot >> 16) > 0) throw context.runtime.newArgumentError("unknown keyword: " + keyAsString);
// Could not find it anywhere.
if (((short) (slot & 0xffff)) < 0) throw context.runtime.newArgumentError("unknown keyword: " + keyAsString);
}
};

public static IRubyObject match3(ThreadContext context, RubyRegexp regexp, IRubyObject argValue) {
if (argValue instanceof RubyString) {
return regexp.op_match19(context, argValue);
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -1074,16 +1074,17 @@ public void CheckArityInstr(CheckArityInstr checkarityinstr) {
if (jvm.methodData().specificArity >= 0) {
// no arity check in specific arity path
} else {
jvmMethod().loadRuntime();
jvmMethod().loadContext();
jvmMethod().loadStaticScope();
jvmMethod().loadArgs();
// TODO: pack these, e.g. in a constant pool String
jvmAdapter().ldc(checkarityinstr.required);
jvmAdapter().ldc(checkarityinstr.opt);
jvmAdapter().ldc(checkarityinstr.rest);
jvmAdapter().ldc(checkarityinstr.receivesKeywords);
jvmAdapter().ldc(checkarityinstr.restKey);
jvmMethod().loadBlockType();
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkArity", sig(void.class, Ruby.class, StaticScope.class, Object[].class, int.class, int.class, boolean.class, boolean.class, int.class, Block.Type.class));
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkArity", sig(void.class, ThreadContext.class, StaticScope.class, Object[].class, int.class, int.class, boolean.class, boolean.class, int.class, Block.Type.class));
}
}

19 changes: 10 additions & 9 deletions core/src/main/java/org/jruby/java/proxies/JavaProxy.java
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@
import org.jruby.RubyArray;
import org.jruby.RubyClass;
import org.jruby.RubyHash;
import org.jruby.RubyHash.Visitor;
import org.jruby.RubyMethod;
import org.jruby.RubyModule;
import org.jruby.RubyObject;
@@ -180,18 +179,13 @@ protected Object cloneObject() {
/**
* Create a name/newname map of fields to be exposed as methods.
*/
private static Map<String, String> getFieldListFromArgs(final IRubyObject[] args) {
private static Map<String, String> getFieldListFromArgs(ThreadContext context, IRubyObject[] args) {
final HashMap<String, String> map = new HashMap<>(args.length, 1);
// Get map of all fields we want to define.
for (int i = 0; i < args.length; i++) {
final IRubyObject arg = args[i];
if ( arg instanceof RubyHash ) {
((RubyHash) arg).visitAll(new Visitor() {
@Override
public void visit(IRubyObject key, IRubyObject value) {
map.put(key.asString().toString(), value.asString().toString());
}
});
((RubyHash) arg).visitAll(context, MapPopulatorVisitor, map);
} else {
String value = arg.asString().toString();
map.put(value, value);
@@ -200,6 +194,13 @@ public void visit(IRubyObject key, IRubyObject value) {
return map;
}

private static final RubyHash.VisitorWithState<Map> MapPopulatorVisitor = new RubyHash.VisitorWithState<Map>() {
@Override
public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Map map) {
map.put(key.asString().toString(), value.asString().toString());
}
};

// Look through all mappings to find a match entry for this field
private static void installField(final ThreadContext context,
final Map<String, String> fieldMap, final Field field,
@@ -249,7 +250,7 @@ private static void installField(final ThreadContext context,
private static void findFields(final ThreadContext context,
final RubyModule topModule, final IRubyObject[] args,
final boolean asReader, final boolean asWriter) {
final Map<String, String> fieldMap = getFieldListFromArgs(args);
final Map<String, String> fieldMap = getFieldListFromArgs(context, args);

for (RubyModule module = topModule; module != null; module = module.getSuperClass()) {
final Class<?> javaClass = JavaClass.getJavaClassIfProxy(context, module);
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java
Original file line number Diff line number Diff line change
@@ -251,16 +251,17 @@ public RubyHashEntry internalDeleteEntry(final RubyHashEntry entry) {
}

@Override
public void visitAll(Visitor visitor) {
public <T> void visitAll(ThreadContext context, VisitorWithState visitor, T state) {
final Ruby runtime = getRuntime();
// NOTE: this is here to make maps act similar to Hash-es which allow modifications while
// iterating (meant from the same thread) ... thus we avoid iterating entrySet() directly
final Map<Object, Object> map = mapDelegate();
final Map.Entry[] entries = map.entrySet().toArray( new Map.Entry[map.size() ] );
int index = 0;
for ( Map.Entry entry : entries ) {
IRubyObject key = JavaUtil.convertJavaToUsableRubyObject(runtime, entry.getKey());
IRubyObject value = JavaUtil.convertJavaToUsableRubyObject(runtime, entry.getValue());
visitor.visit(key, value);
visitor.visit(context, this, key, value, index++, state);
}
}