Skip to content

Commit

Permalink
[Truffle] Fixed Array#{inject,reduce} without an initial value.
Browse files Browse the repository at this point in the history
@nirvdrum
nirvdrum committed Jun 26, 2015
1 parent fd3a43e commit 2ccf01e
Showing 1 changed file with 104 additions and 39 deletions.
143 changes: 104 additions & 39 deletions truffle/src/main/java/org/jruby/truffle/nodes/core/array/ArrayNodes.java
View file
Original file line number Diff line number Diff line change
@@ -2084,16 +2084,112 @@ public InjectNode(RubyContext context, SourceSection sourceSection) {
dispatch = DispatchHeadNodeFactory.createMethodCall(context, MissingBehavior.CALL_METHOD_MISSING);
}

@Specialization(guards = "isIntArray(array)")
@Specialization(guards = { "isIntArray(array)", "wasProvided(initial)" })
public Object injectIntegerFixnum(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return injectIntegerFixnumHelper(frame, array, initial, block, 0);
}

@Specialization(guards = { "isIntArray(array)", "wasNotProvided(initial)" })
public Object injectIntegerFixnumNoInitial(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
if (getSize(array) > 0) {

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jun 26, 2015

Contributor

I would make this a guard using isEmptyArray.

This comment has been minimized.

Sign in to view

Copy link
@nirvdrum

nirvdrum Jun 26, 2015

Author Contributor

Easy enough to do. I was just trying to avoid specialization explosion. Although it looks like only 4 new cases and they shouldn't grow.
final int[] store = (int[]) getStore(array);

return injectIntegerFixnumHelper(frame, array, store[0], block, 1);
}

return nil();
}

@Specialization(guards = { "isLongArray(array)", "wasProvided(initial)" })
public Object injectLongFixnum(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return injectLongFixnumHelper(frame, array, initial, block, 0);
}

@Specialization(guards = { "isLongArray(array)", "wasNotProvided(initial)" })
public Object injectLongFixnumNoInitial(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
if (getSize(array) > 0) {

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jun 26, 2015

Contributor

Guard
final long[] store = (long[]) getStore(array);

return injectLongFixnumHelper(frame, array, store[0], block, 1);
}

return nil();
}

@Specialization(guards = { "isDoubleArray(array)", "wasProvided(initial)" })
public Object injectFloat(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return injectFloatHelper(frame, array, initial, block, 0);
}

@Specialization(guards = { "isDoubleArray(array)", "wasNotProvided(initial)" })
public Object injectFloatNoInitial(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
if (getSize(array) > 0) {

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jun 26, 2015

Contributor

Guard
final double[] store = (double[]) getStore(array);

return injectFloatHelper(frame, array, store[0], block, 1);
}

return nil();
}

@Specialization(guards = { "isObjectArray(array)", "wasProvided(initial)" })
public Object injectObject(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return injectObjectHelper(frame, array, initial, block, 0);
}

@Specialization(guards = { "isObjectArray(array)", "wasNotProvided(initial)" })
public Object injectObjectNoInitial(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
if (getSize(array) > 0) {

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jun 26, 2015

Contributor

Guard
final Object[] store = (Object[]) getStore(array);

return injectObjectHelper(frame, array, store[0], block, 1);
}

return nil();
}

@Specialization(guards = { "isNullArray(array)", "wasProvided(initial)" })
public Object injectNull(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return initial;
}

@Specialization(guards = { "isNullArray(array)", "wasNotProvided(initial)" })
public Object injectNullNoInitial(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return nil();
}

@Specialization(guards = "isRubySymbol(symbol)")
public Object inject(VirtualFrame frame, RubyArray array, RubyBasicObject symbol, NotProvided block) {
CompilerDirectives.transferToInterpreter();

final Object[] store = slowToArray(array);

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jun 26, 2015

Contributor

If this is new code can you find a way to write this without slowToArray and without the transfer? I'm trying to remove all the slow cases from Array and Hash so we should avoid adding more.

This comment has been minimized.

Sign in to view

Copy link
@nirvdrum

nirvdrum Jun 26, 2015

Author Contributor

It's existing code. The diff just looks weird because I removed a lot of code above it and placed it into helpers below it. But I can look at removing that.

if (store.length < 2) {
if (store.length == 1) {
return store[0];
} else {
return getContext().getCoreLibrary().getNilObject();
}
}

Object accumulator = dispatch.call(frame, store[0], symbol, null, store[1]);

for (int n = 2; n < getSize(array); n++) {
accumulator = dispatch.call(frame, accumulator, symbol, null, store[n]);
}

return accumulator;
}

private Object injectIntegerFixnumHelper(VirtualFrame frame, RubyArray array, Object initial, RubyProc block, int startIndex) {
int count = 0;

final int[] store = (int[]) getStore(array);

Object accumulator = initial;

try {
for (int n = 0; n < getSize(array); n++) {
for (int n = startIndex; n < getSize(array); n++) {
if (CompilerDirectives.inInterpreter()) {
count++;
}
@@ -2109,16 +2205,15 @@ public Object injectIntegerFixnum(VirtualFrame frame, RubyArray array, Object in
return accumulator;
}

@Specialization(guards = "isLongArray(array)")
public Object injectLongFixnum(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
private Object injectLongFixnumHelper(VirtualFrame frame, RubyArray array, Object initial, RubyProc block, int startIndex) {
int count = 0;

final long[] store = (long[]) getStore(array);

Object accumulator = initial;

try {
for (int n = 0; n < getSize(array); n++) {
for (int n = startIndex; n < getSize(array); n++) {
if (CompilerDirectives.inInterpreter()) {
count++;
}
@@ -2134,16 +2229,15 @@ public Object injectLongFixnum(VirtualFrame frame, RubyArray array, Object initi
return accumulator;
}

@Specialization(guards = "isDoubleArray(array)")
public Object injectFloat(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
private Object injectFloatHelper(VirtualFrame frame, RubyArray array, Object initial, RubyProc block, int startIndex) {
int count = 0;

final double[] store = (double[]) getStore(array);

Object accumulator = initial;

try {
for (int n = 0; n < getSize(array); n++) {
for (int n = startIndex; n < getSize(array); n++) {
if (CompilerDirectives.inInterpreter()) {
count++;
}
@@ -2159,16 +2253,15 @@ public Object injectFloat(VirtualFrame frame, RubyArray array, Object initial, R
return accumulator;
}

@Specialization(guards = "isObjectArray(array)")
public Object injectObject(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
private Object injectObjectHelper(VirtualFrame frame, RubyArray array, Object initial, RubyProc block, int startIndex) {
int count = 0;

final Object[] store = (Object[]) getStore(array);

Object accumulator = initial;

try {
for (int n = 0; n < getSize(array); n++) {
for (int n = startIndex; n < getSize(array); n++) {
if (CompilerDirectives.inInterpreter()) {
count++;
}
@@ -2184,34 +2277,6 @@ public Object injectObject(VirtualFrame frame, RubyArray array, Object initial,
return accumulator;
}

@Specialization(guards = "isNullArray(array)")
public Object injectNull(VirtualFrame frame, RubyArray array, Object initial, RubyProc block) {
return initial;
}

@Specialization(guards = "isRubySymbol(symbol)")
public Object inject(VirtualFrame frame, RubyArray array, RubyBasicObject symbol, NotProvided block) {
CompilerDirectives.transferToInterpreter();

final Object[] store = slowToArray(array);

if (store.length < 2) {
if (store.length == 1) {
return store[0];
} else {
return getContext().getCoreLibrary().getNilObject();
}
}

Object accumulator = dispatch.call(frame, store[0], symbol, null, store[1]);

for (int n = 2; n < getSize(array); n++) {
accumulator = dispatch.call(frame, accumulator, symbol, null, store[n]);
}

return accumulator;
}

}

@CoreMethod(names = "insert", required = 1, raiseIfFrozenSelf = true, argumentsAsArray = true)

1 comment on commit 2ccf01e

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chrisseaton Could you please spot check this for me and make sure I didn't do anything that would hurt performance?

Please sign in to comment.