Skip to content

Commit

Permalink
[Truffle] Options shouldn't be static fields.
Browse files Browse the repository at this point in the history
@chrisseaton
chrisseaton committed Jan 29, 2016
1 parent 4ec1a7c commit ecf4c37
Showing 17 changed files with 33 additions and 27 deletions.
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/language/globals/ReadGlobalVariableNode.java
View file
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ public class ReadGlobalVariableNode extends RubyNode {
public ReadGlobalVariableNode(RubyContext context, SourceSection sourceSection, String name) {
super(context, sourceSection);
this.globalVariablesObject = context.getCoreLibrary().getGlobalVariablesObject();
readNode = ReadHeadObjectFieldNodeGen.create(name, nil());
readNode = ReadHeadObjectFieldNodeGen.create(getContext(), name, nil());
}

@Override
2 changes: 1 addition & 1 deletion ...e/src/main/java/org/jruby/truffle/language/globals/ReadThreadLocalGlobalVariableNode.java
View file
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ public class ReadThreadLocalGlobalVariableNode extends RubyNode {
public ReadThreadLocalGlobalVariableNode(RubyContext context, SourceSection sourceSection, String name) {
super(context, sourceSection);
this.threadLocalVariablesObjectNode = new ThreadLocalObjectNode(context, sourceSection);
readNode = ReadHeadObjectFieldNodeGen.create(name, nil());
readNode = ReadHeadObjectFieldNodeGen.create(getContext(), name, nil());
}

@Override
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/language/globals/WriteGlobalVariableNode.java
View file
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ public WriteGlobalVariableNode(RubyContext context, SourceSection sourceSection,
super(context, sourceSection);
this.globalVariablesObject = context.getCoreLibrary().getGlobalVariablesObject();
this.rhs = rhs;
writeNode = WriteHeadObjectFieldNodeGen.create(name);
writeNode = WriteHeadObjectFieldNodeGen.create(getContext(), name);
}

@Override
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/core/ExceptionNodes.java
View file
Original file line number Diff line number Diff line change
@@ -84,7 +84,7 @@ public abstract static class BacktraceNode extends CoreMethodArrayArgumentsNode

public BacktraceNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
readCustomBacktrace = ReadHeadObjectFieldNodeGen.create("@custom_backtrace", null);
readCustomBacktrace = ReadHeadObjectFieldNodeGen.create(getContext(), "@custom_backtrace", null);
}

@Specialization
10 changes: 5 additions & 5 deletions truffle/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
View file
Original file line number Diff line number Diff line change
@@ -998,11 +998,11 @@ protected String checkName(String name) {
}

protected ReadHeadObjectFieldNode createReadFieldNode(String name) {
return ReadHeadObjectFieldNodeGen.create(name, nil());
return ReadHeadObjectFieldNodeGen.create(getContext(), name, nil());
}

protected int getCacheLimit() {
return Options.INSTANCE_VARIABLE_LOOKUP_CACHE;
return getContext().getOptions().INSTANCE_VARIABLE_LOOKUP_CACHE;
}

}
@@ -1060,11 +1060,11 @@ protected String checkName(String name) {
}

protected WriteHeadObjectFieldNode createWriteFieldNode(String name) {
return WriteHeadObjectFieldNodeGen.create(name);
return WriteHeadObjectFieldNodeGen.create(getContext(), name);
}

protected int getCacheLimit() {
return Options.INSTANCE_VARIABLE_LOOKUP_CACHE;
return getContext().getOptions().INSTANCE_VARIABLE_LOOKUP_CACHE;
}

}
@@ -2084,7 +2084,7 @@ public UntaintNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
isFrozenNode = IsFrozenNodeGen.create(context, sourceSection, null);
isTaintedNode = IsTaintedNodeGen.create(context, sourceSection, null);
writeTaintNode = WriteHeadObjectFieldNodeGen.create(Layouts.TAINTED_IDENTIFIER);
writeTaintNode = WriteHeadObjectFieldNodeGen.create(getContext(), Layouts.TAINTED_IDENTIFIER);
}

@Specialization
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/core/ObjectSpaceNodes.java
View file
Original file line number Diff line number Diff line change
@@ -86,7 +86,7 @@ public double id2RefFloat(DynamicObject id) {
}

protected ReadHeadObjectFieldNode createReadObjectIDNode() {
return ReadHeadObjectFieldNodeGen.create(Layouts.OBJECT_ID_IDENTIFIER, 0L);
return ReadHeadObjectFieldNodeGen.create(getContext(), Layouts.OBJECT_ID_IDENTIFIER, 0L);
}

protected boolean isLargeFixnumID(DynamicObject id) {
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/objects/FreezeNode.java
View file
Original file line number Diff line number Diff line change
@@ -71,7 +71,7 @@ public Object freezeSymbol(DynamicObject symbol) {
public Object freeze(DynamicObject object) {
if (writeFrozenNode == null) {
CompilerDirectives.transferToInterpreter();
writeFrozenNode = insert(WriteHeadObjectFieldNodeGen.create(Layouts.FROZEN_IDENTIFIER));
writeFrozenNode = insert(WriteHeadObjectFieldNodeGen.create(getContext(), Layouts.FROZEN_IDENTIFIER));
}

writeFrozenNode.execute(object, true);
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/objects/IsFrozenNode.java
View file
Original file line number Diff line number Diff line change
@@ -57,6 +57,6 @@ protected boolean isFrozen(DynamicObject object,
}

protected ReadHeadObjectFieldNode createReadFrozenNode() {
return ReadHeadObjectFieldNodeGen.create(Layouts.FROZEN_IDENTIFIER, false);
return ReadHeadObjectFieldNodeGen.create(getContext(), Layouts.FROZEN_IDENTIFIER, false);
}
}
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/objects/IsTaintedNode.java
View file
Original file line number Diff line number Diff line change
@@ -58,7 +58,7 @@ protected boolean isTainted(DynamicObject object,
}

protected ReadHeadObjectFieldNode createReadTaintedNode() {
return ReadHeadObjectFieldNodeGen.create(Layouts.TAINTED_IDENTIFIER, false);
return ReadHeadObjectFieldNodeGen.create(getContext(), Layouts.TAINTED_IDENTIFIER, false);
}

}
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/objects/ReadInstanceVariableNode.java
View file
Original file line number Diff line number Diff line change
@@ -30,7 +30,7 @@ public class ReadInstanceVariableNode extends RubyNode {
public ReadInstanceVariableNode(RubyContext context, SourceSection sourceSection, String name, RubyNode receiver) {
super(context, sourceSection);
this.receiver = receiver;
readNode = ReadHeadObjectFieldNodeGen.create(name, nil());
readNode = ReadHeadObjectFieldNodeGen.create(getContext(), name, nil());
}

@Override
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/objects/TaintNode.java
View file
Original file line number Diff line number Diff line change
@@ -61,7 +61,7 @@ public Object taintSymbol(DynamicObject symbol) {
public Object taint(DynamicObject object) {
if (writeTaintNode == null) {
CompilerDirectives.transferToInterpreter();
writeTaintNode = insert(WriteHeadObjectFieldNodeGen.create(Layouts.TAINTED_IDENTIFIER));
writeTaintNode = insert(WriteHeadObjectFieldNodeGen.create(getContext(), Layouts.TAINTED_IDENTIFIER));
}
writeTaintNode.execute(object, true);
return object;
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/nodes/objects/WriteInstanceVariableNode.java
View file
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ public WriteInstanceVariableNode(RubyContext context, SourceSection sourceSectio
super(context, sourceSection);
this.receiver = receiver;
this.rhs = rhs;
writeNode = WriteHeadObjectFieldNodeGen.create(name);
writeNode = WriteHeadObjectFieldNodeGen.create(getContext(), name);
}

@Override
7 changes: 5 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/nodes/objectstorage/ReadHeadObjectFieldNode.java
View file
Original file line number Diff line number Diff line change
@@ -19,13 +19,16 @@
import com.oracle.truffle.api.object.Shape;
import org.jruby.truffle.nodes.ShapeCachingGuards;
import org.jruby.truffle.runtime.Options;
import org.jruby.truffle.runtime.RubyContext;

@ImportStatic(ShapeCachingGuards.class)
public abstract class ReadHeadObjectFieldNode extends Node {
private final RubyContext context;
private final Object defaultValue;
protected final Object name;

public ReadHeadObjectFieldNode(Object name, Object defaultValue) {
public ReadHeadObjectFieldNode(RubyContext context, Object name, Object defaultValue) {
this.context = context;
this.name = name;
this.defaultValue = defaultValue;
}
@@ -62,7 +65,7 @@ protected Object readObjectFieldUncached(DynamicObject receiver) {
}

protected int getCacheLimit() {
return Options.INSTANCE_VARIABLE_LOOKUP_CACHE;
return context.getOptions().INSTANCE_VARIABLE_LOOKUP_CACHE;
}

}
7 changes: 5 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/nodes/objectstorage/WriteHeadObjectFieldNode.java
View file
Original file line number Diff line number Diff line change
@@ -19,13 +19,16 @@
import com.oracle.truffle.api.object.*;
import org.jruby.truffle.nodes.ShapeCachingGuards;
import org.jruby.truffle.runtime.Options;
import org.jruby.truffle.runtime.RubyContext;

@ImportStatic(ShapeCachingGuards.class)
public abstract class WriteHeadObjectFieldNode extends Node {

private final RubyContext context;
private final Object name;

public WriteHeadObjectFieldNode(Object name) {
public WriteHeadObjectFieldNode(RubyContext context, Object name) {
this.context = context;
this.name = name;
}

@@ -111,7 +114,7 @@ protected Assumption createAssumption() {
}

protected int getCacheLimit() {
return Options.INSTANCE_VARIABLE_LOOKUP_CACHE;
return context.getOptions().INSTANCE_VARIABLE_LOOKUP_CACHE;
}

}
4 changes: 2 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/nodes/rubinius/ObjectPrimitiveNodes.java
View file
Original file line number Diff line number Diff line change
@@ -98,11 +98,11 @@ public long objectID(DynamicObject object,
}

protected ReadHeadObjectFieldNode createReadObjectIDNode() {
return ReadHeadObjectFieldNodeGen.create(Layouts.OBJECT_ID_IDENTIFIER, 0L);
return ReadHeadObjectFieldNodeGen.create(getContext(), Layouts.OBJECT_ID_IDENTIFIER, 0L);
}

protected WriteHeadObjectFieldNode createWriteObjectIDNode() {
return WriteHeadObjectFieldNodeGen.create(Layouts.OBJECT_ID_IDENTIFIER);
return WriteHeadObjectFieldNodeGen.create(getContext(), Layouts.OBJECT_ID_IDENTIFIER);
}

}
8 changes: 4 additions & 4 deletions truffle/src/main/java/org/jruby/truffle/nodes/rubinius/StatPrimitiveNodes.java
View file
Original file line number Diff line number Diff line change
@@ -163,7 +163,7 @@ public static abstract class StatStatPrimitiveNode extends RubiniusPrimitiveArra

public StatStatPrimitiveNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
writeStatNode = WriteHeadObjectFieldNodeGen.create(STAT_IDENTIFIER);
writeStatNode = WriteHeadObjectFieldNodeGen.create(getContext(), STAT_IDENTIFIER);
}

@TruffleBoundary
@@ -193,7 +193,7 @@ public static abstract class StatFStatPrimitiveNode extends RubiniusPrimitiveArr

public StatFStatPrimitiveNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
writeStatNode = WriteHeadObjectFieldNodeGen.create(STAT_IDENTIFIER);
writeStatNode = WriteHeadObjectFieldNodeGen.create(getContext(), STAT_IDENTIFIER);
}

@TruffleBoundary
@@ -218,7 +218,7 @@ public static abstract class StatLStatPrimitiveNode extends RubiniusPrimitiveArr

public StatLStatPrimitiveNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
writeStatNode = WriteHeadObjectFieldNodeGen.create(STAT_IDENTIFIER);
writeStatNode = WriteHeadObjectFieldNodeGen.create(getContext(), STAT_IDENTIFIER);
}

@TruffleBoundary
@@ -247,7 +247,7 @@ public static abstract class StatReadPrimitiveNode extends RubiniusPrimitiveArra

public StatReadPrimitiveNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
readStatNode = ReadHeadObjectFieldNodeGen.create(STAT_IDENTIFIER, null);
readStatNode = ReadHeadObjectFieldNodeGen.create(getContext(), STAT_IDENTIFIER, null);
}

public FileStat getStat(DynamicObject rubyStat) {
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/runtime/Options.java
View file
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ public class Options {
public final int IS_A_CACHE = org.jruby.util.cli.Options.TRUFFLE_IS_A_CACHE.load();
public final int BIND_CACHE = org.jruby.util.cli.Options.TRUFFLE_BIND_CACHE.load();
public final int CONSTANT_LOOKUP_CACHE = org.jruby.util.cli.Options.TRUFFLE_CONSTANT_LOOKUP_CACHE.load();
public static final int INSTANCE_VARIABLE_LOOKUP_CACHE = org.jruby.util.cli.Options.TRUFFLE_INSTANCE_VARIABLE_LOOKUP_CACHE.load();
public final int INSTANCE_VARIABLE_LOOKUP_CACHE = org.jruby.util.cli.Options.TRUFFLE_INSTANCE_VARIABLE_LOOKUP_CACHE.load();
public final int BINDING_LOCAL_VARIABLE_CACHE = org.jruby.util.cli.Options.TRUFFLE_BINDING_LOCAL_VARIABLE_CACHE.load();
public final int SYMBOL_TO_PROC_CACHE = org.jruby.util.cli.Options.TRUFFLE_SYMBOL_TO_PROC_CACHE.load();
public final int ALLOCATE_CLASS_CACHE = org.jruby.util.cli.Options.TRUFFLE_ALLOCATE_CLASS_CACHE.load();

5 comments on commit ecf4c37

@eregon
Copy link
Member

@eregon eregon commented on ecf4c37 Feb 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose?
Do we really have a use-case for different values per PolyglotEngine for caches? Or would we even specify them?

@eregon
Copy link
Member

@eregon eregon commented on ecf4c37 Feb 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The less nodes using the context the better IMHO, and this is just going against it)

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just seemed to me that it was something logical to tidy up - removing static state where possible. I think if we do some complex multi-tennant stuff in the future then we may want to set some options per-context.

But even if you disagreed, it needs to be the same for all the options, not just one of them.

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think it's a limitation of JRuby when used with Nailgun/Drip that you can't change options. We'd like to be able to do that if we use a similar approach.

Sorry, something went wrong.

@eregon
Copy link
Member

@eregon eregon commented on ecf4c37 Feb 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change it for all CACHE options, but consistency wins and Nailgun/Drip seems compelling and realistic examples indeed for tuning some options.

Sorry, something went wrong.

Please sign in to comment.