From a68668a24f746acbcd6c352efb07e7a8d61d4821 Mon Sep 17 00:00:00 2001 From: ccocchi Date: Sun, 24 Oct 2021 23:43:35 +0200 Subject: [PATCH 1/8] Ruby part --- src/main/ruby/truffleruby/core/array.rb | 54 +++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/main/ruby/truffleruby/core/array.rb b/src/main/ruby/truffleruby/core/array.rb index c035ddcd27a2..c940b84b36f9 100644 --- a/src/main/ruby/truffleruby/core/array.rb +++ b/src/main/ruby/truffleruby/core/array.rb @@ -146,6 +146,60 @@ def ==(other) true end + alias_method :java_slice, :[] + + def slice(start_or_range_or_sequence, *args) + if Primitive.object_kind_of?(start_or_range_or_sequence, Enumerator::ArithmeticSequence) + seq = start_or_range_or_sequence + len = size + + if seq.step < 0 # inverse range with negative step + start = seq.end + stop = seq.begin + step = seq.step + else + start = seq.begin + stop = seq.end + step = seq.step + end + + start ||= 0 # begin-less range + stop ||= -1 # endless range + + start += len if start < 0 + stop += len if stop < 0 + stop -= 1 if seq.exclude_end? + + if start < 0 || start > len + raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 + return nil + end + + diff = stop - start + return [] if diff < 0 + + ustep = step.abs + nlen = (diff + ustep) / ustep + i = 0 + j = start + (step > 0 ? 0 : diff) # because we inverted negative step ranges + res = Array.new(nlen) + + while i < nlen + res[i] = self[j] + i += 1 + j += step + end + + res + else + java_slice(start_or_range_or_sequence, *args) + end + end + + def [](*args) + slice(*args) + end + def assoc(obj) each do |x| if Primitive.object_kind_of?(x, Array) and x.first == obj From f5327848eb81277c6099c8f3861b393414cbb818 Mon Sep 17 00:00:00 2001 From: ccocchi Date: Mon, 25 Oct 2021 01:29:29 +0200 Subject: [PATCH 2/8] Ruby is weird sometimes --- src/main/ruby/truffleruby/core/array.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/ruby/truffleruby/core/array.rb b/src/main/ruby/truffleruby/core/array.rb index c940b84b36f9..a18aa350ca6e 100644 --- a/src/main/ruby/truffleruby/core/array.rb +++ b/src/main/ruby/truffleruby/core/array.rb @@ -168,15 +168,22 @@ def slice(start_or_range_or_sequence, *args) start += len if start < 0 stop += len if stop < 0 - stop -= 1 if seq.exclude_end? if start < 0 || start > len raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 return nil end + stop -= 1 if seq.exclude_end? diff = stop - start + return [] if diff < 0 + return self[start, 1] if (step > 0 && step > diff) || (step < 0 && step < -diff) + + if diff >= len + raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 + diff = len - start - 1 + end ustep = step.abs nlen = (diff + ustep) / ustep From 339f6fdca5f1bb6f21c07ceee332a93964bf785b Mon Sep 17 00:00:00 2001 From: ccocchi Date: Mon, 25 Oct 2021 02:28:34 +0200 Subject: [PATCH 3/8] The small differences make the biggest changes --- src/main/ruby/truffleruby/core/array.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/ruby/truffleruby/core/array.rb b/src/main/ruby/truffleruby/core/array.rb index a18aa350ca6e..13f77925de26 100644 --- a/src/main/ruby/truffleruby/core/array.rb +++ b/src/main/ruby/truffleruby/core/array.rb @@ -174,21 +174,23 @@ def slice(start_or_range_or_sequence, *args) return nil end - stop -= 1 if seq.exclude_end? + stop += 1 unless seq.exclude_end? + stop = len if (step == -1 || step == 1) && stop > len + diff = stop - start - return [] if diff < 0 + return [] if diff <= 0 return self[start, 1] if (step > 0 && step > diff) || (step < 0 && step < -diff) - if diff >= len + if diff > len raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 - diff = len - start - 1 + diff = len - start end ustep = step.abs - nlen = (diff + ustep) / ustep + nlen = (diff + ustep - 1) / ustep i = 0 - j = start + (step > 0 ? 0 : diff) # because we inverted negative step ranges + j = start + (step > 0 ? 0 : diff - 1) # because we inverted negative step ranges res = Array.new(nlen) while i < nlen From 38bfb9b0953f25f1c771162136a5d569bdcbde00 Mon Sep 17 00:00:00 2001 From: ccocchi Date: Sat, 30 Oct 2021 11:52:38 +0200 Subject: [PATCH 4/8] Specialize slice node --- spec/ruby/core/array/shared/slice.rb | 16 ++++ .../org/truffleruby/core/CoreLibrary.java | 2 + .../truffleruby/core/array/ArrayNodes.java | 6 ++ .../org/truffleruby/language/RubyGuards.java | 4 + src/main/ruby/truffleruby/core/array.rb | 86 ++++++++----------- 5 files changed, 66 insertions(+), 48 deletions(-) diff --git a/spec/ruby/core/array/shared/slice.rb b/spec/ruby/core/array/shared/slice.rb index cf195ad7a46b..2e8e124b3e82 100644 --- a/spec/ruby/core/array/shared/slice.rb +++ b/spec/ruby/core/array/shared/slice.rb @@ -743,6 +743,22 @@ def to.to_int() -2 end @array.send(@method, eval("(-2..-4).step(10)")).should == [] @array.send(@method, eval("(-2...-4).step(10)")).should == [] end + + it "foos" do + @array.send(@method, eval("(0..6).step(1)")).should == [0, 1, 2, 3, 4, 5] + -> { @array.send(@method, eval("(0..6).step(2)")) }.should raise_error(RangeError, /out of range/) + + @array.send(@method, eval("(1..6).step(2)")).should == [1, 3, 5] + @array.send(@method, eval("(2..7).step(2)")).should == [2, 4] + -> { @array.send(@method, eval("(2..8).step(2)")) }.should raise_error(RangeError, /out of range/) + + @array.send(@method, eval("(6..1).step(-2)")).should == [5, 3, 1] + @array.send(@method, eval("(7..2).step(-2)")).should == [5, 3] + -> { @array.send(@method, eval("(8..2).step(-2)")) }.should raise_error(RangeError, /out of range/) + + @array.send(@method, eval("(6..).step(1)")).should == [] + @array.send(@method, eval("(7..).step(1)")).should == nil + end end end diff --git a/src/main/java/org/truffleruby/core/CoreLibrary.java b/src/main/java/org/truffleruby/core/CoreLibrary.java index 37e8ac223782..29983ff2b1d5 100644 --- a/src/main/java/org/truffleruby/core/CoreLibrary.java +++ b/src/main/java/org/truffleruby/core/CoreLibrary.java @@ -180,6 +180,7 @@ public class CoreLibrary { public final RubyClass zeroDivisionErrorClass; public final RubyModule enumerableModule; public final RubyClass enumeratorClass; + public final RubyClass arithmeticSequenceClass; public final RubyModule errnoModule; public final RubyModule kernelModule; public final RubyModule truffleFFIModule; @@ -398,6 +399,7 @@ public CoreLibrary(RubyContext context, RubyLanguage language) { dirClass = defineClass("Dir"); encodingClass = defineClass("Encoding"); enumeratorClass = defineClass("Enumerator"); + arithmeticSequenceClass = defineClass("ArithmeticSequence"); falseClass = defineClass("FalseClass"); fiberClass = defineClass("Fiber"); defineModule("FileTest"); diff --git a/src/main/java/org/truffleruby/core/array/ArrayNodes.java b/src/main/java/org/truffleruby/core/array/ArrayNodes.java index d8372af42023..e3421087f302 100644 --- a/src/main/java/org/truffleruby/core/array/ArrayNodes.java +++ b/src/main/java/org/truffleruby/core/array/ArrayNodes.java @@ -282,6 +282,12 @@ protected Object indexRange(RubyArray array, RubyRange range, NotProvided length return readSlice.executeReadSlice(array, startLength[0], len); } + @Specialization(guards = { "isRubyArithmeticSequence(index)" }) + protected Object indexArithmeticSequence(RubyArray array, Object index, NotProvided length, + @Cached DispatchNode callSliceArithmeticSequence) { + return callSliceArithmeticSequence.call(array, "slice_arithmetic_sequence", index); + } + @Specialization(guards = { "!isInteger(index)", "!isRubyRange(index)" }) protected Object indexFallback(RubyArray array, Object index, NotProvided length, @Cached AtNode accessWithIndexConversion) { diff --git a/src/main/java/org/truffleruby/language/RubyGuards.java b/src/main/java/org/truffleruby/language/RubyGuards.java index 8de7a212b3bf..2eabf982f0d6 100644 --- a/src/main/java/org/truffleruby/language/RubyGuards.java +++ b/src/main/java/org/truffleruby/language/RubyGuards.java @@ -169,6 +169,10 @@ public static boolean isNil(Object object) { return object == Nil.INSTANCE; } + public static boolean isRubyArithmeticSequence(Object object) { + return true; + } + // Internal types public static boolean isRubyDynamicObject(Object object) { diff --git a/src/main/ruby/truffleruby/core/array.rb b/src/main/ruby/truffleruby/core/array.rb index 13f77925de26..4dc94a8001af 100644 --- a/src/main/ruby/truffleruby/core/array.rb +++ b/src/main/ruby/truffleruby/core/array.rb @@ -146,67 +146,57 @@ def ==(other) true end - alias_method :java_slice, :[] + private def slice_arithmetic_sequence(seq) + len = size - def slice(start_or_range_or_sequence, *args) - if Primitive.object_kind_of?(start_or_range_or_sequence, Enumerator::ArithmeticSequence) - seq = start_or_range_or_sequence - len = size - - if seq.step < 0 # inverse range with negative step - start = seq.end - stop = seq.begin - step = seq.step - else - start = seq.begin - stop = seq.end - step = seq.step - end + if seq.step < 0 # inverse range with negative step + start = seq.end + stop = seq.begin + step = seq.step + else + start = seq.begin + stop = seq.end + step = seq.step + end - start ||= 0 # begin-less range - stop ||= -1 # endless range + start ||= 0 # begin-less range + stop ||= -1 # endless range - start += len if start < 0 - stop += len if stop < 0 + # negative indexes refer to end of array + start += len if start < 0 + stop += len if stop < 0 - if start < 0 || start > len - raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 - return nil - end + if start < 0 || start > len + raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 + return nil + end - stop += 1 unless seq.exclude_end? - stop = len if (step == -1 || step == 1) && stop > len + stop += 1 unless seq.exclude_end? + diff = stop - start - diff = stop - start + raise RangeError, "#{seq.inspect} out of range" if diff > len && (step < -1 || step > 1) + return [] if diff <= 0 - return [] if diff <= 0 - return self[start, 1] if (step > 0 && step > diff) || (step < 0 && step < -diff) + diff = len - start if (len < diff || len < start + diff) - if diff > len - raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 - diff = len - start - end + return self[start, diff] if step == 1 # step == 1 is a simple slice - ustep = step.abs - nlen = (diff + ustep - 1) / ustep - i = 0 - j = start + (step > 0 ? 0 : diff - 1) # because we inverted negative step ranges - res = Array.new(nlen) + # optimize when no step will be done and only start element is returned + return self[start, 1] if (step > 0 && step > diff) || (step < 0 && step < -diff) - while i < nlen - res[i] = self[j] - i += 1 - j += step - end + ustep = step.abs + nlen = (diff + ustep - 1) / ustep + i = 0 + j = start + (step > 0 ? 0 : diff - 1) # because we inverted negative step ranges + res = Array.new(nlen) - res - else - java_slice(start_or_range_or_sequence, *args) + while i < nlen + res[i] = self[j] + i += 1 + j += step end - end - def [](*args) - slice(*args) + res end def assoc(obj) From dd89846f3f0a72b0590e2828dd1ddd26639e1876 Mon Sep 17 00:00:00 2001 From: ccocchi Date: Sun, 31 Oct 2021 00:19:09 +0200 Subject: [PATCH 5/8] Use isANode --- spec/ruby/core/array/shared/slice.rb | 30 ++++++++++++------- spec/tags/core/array/slice_tags.txt | 6 ---- .../org/truffleruby/core/CoreLibrary.java | 2 +- .../truffleruby/core/array/ArrayNodes.java | 13 ++++++-- .../org/truffleruby/language/RubyGuards.java | 4 --- 5 files changed, 31 insertions(+), 24 deletions(-) delete mode 100644 spec/tags/core/array/slice_tags.txt diff --git a/spec/ruby/core/array/shared/slice.rb b/spec/ruby/core/array/shared/slice.rb index 2e8e124b3e82..540a050130cc 100644 --- a/spec/ruby/core/array/shared/slice.rb +++ b/spec/ruby/core/array/shared/slice.rb @@ -744,20 +744,28 @@ def to.to_int() -2 end @array.send(@method, eval("(-2...-4).step(10)")).should == [] end - it "foos" do - @array.send(@method, eval("(0..6).step(1)")).should == [0, 1, 2, 3, 4, 5] - -> { @array.send(@method, eval("(0..6).step(2)")) }.should raise_error(RangeError, /out of range/) - - @array.send(@method, eval("(1..6).step(2)")).should == [1, 3, 5] - @array.send(@method, eval("(2..7).step(2)")).should == [2, 4] - -> { @array.send(@method, eval("(2..8).step(2)")) }.should raise_error(RangeError, /out of range/) - - @array.send(@method, eval("(6..1).step(-2)")).should == [5, 3, 1] - @array.send(@method, eval("(7..2).step(-2)")).should == [5, 3] - -> { @array.send(@method, eval("(8..2).step(-2)")) }.should raise_error(RangeError, /out of range/) + it "has range with bounds outside of array" do + # end is equal to array's length + @array.send(@method, (0..6).step(1)).should == [0, 1, 2, 3, 4, 5] + -> { @array.send(@method, (0..6).step(2)) }.should raise_error(RangeError) + + # end is greater than length with positive steps + @array.send(@method, (1..6).step(2)).should == [1, 3, 5] + @array.send(@method, (2..7).step(2)).should == [2, 4] + -> { @array.send(@method, (2..8).step(2)) }.should raise_error(RangeError) + + # begin is greater than length with negative steps + @array.send(@method, (6..1).step(-2)).should == [5, 3, 1] + @array.send(@method, (7..2).step(-2)).should == [5, 3] + -> { @array.send(@method, (8..2).step(-2)) }.should raise_error(RangeError) + end + it "has endless range with start outside of array's bounds" do @array.send(@method, eval("(6..).step(1)")).should == [] @array.send(@method, eval("(7..).step(1)")).should == nil + + @array.send(@method, eval("(6..).step(2)")).should == [] + -> { @array.send(@method, eval("(7..).step(2)")) }.should raise_error(RangeError) end end end diff --git a/spec/tags/core/array/slice_tags.txt b/spec/tags/core/array/slice_tags.txt deleted file mode 100644 index e108c5f16c60..000000000000 --- a/spec/tags/core/array/slice_tags.txt +++ /dev/null @@ -1,6 +0,0 @@ -fails:Array#slice can be sliced with Enumerator::ArithmeticSequence has endless range and positive steps -fails:Array#slice can be sliced with Enumerator::ArithmeticSequence has beginless range and positive steps -fails:Array#slice can be sliced with Enumerator::ArithmeticSequence has endless range and negative steps -fails:Array#slice can be sliced with Enumerator::ArithmeticSequence has closed range and positive steps -fails:Array#slice can be sliced with Enumerator::ArithmeticSequence has closed range and negative steps -fails:Array#slice can be sliced with Enumerator::ArithmeticSequence has inverted closed range and positive steps diff --git a/src/main/java/org/truffleruby/core/CoreLibrary.java b/src/main/java/org/truffleruby/core/CoreLibrary.java index 29983ff2b1d5..e0abf0a5f703 100644 --- a/src/main/java/org/truffleruby/core/CoreLibrary.java +++ b/src/main/java/org/truffleruby/core/CoreLibrary.java @@ -399,7 +399,7 @@ public CoreLibrary(RubyContext context, RubyLanguage language) { dirClass = defineClass("Dir"); encodingClass = defineClass("Encoding"); enumeratorClass = defineClass("Enumerator"); - arithmeticSequenceClass = defineClass("ArithmeticSequence"); + arithmeticSequenceClass = defineClass(enumeratorClass, enumeratorClass, "ArithmeticSequence"); falseClass = defineClass("FalseClass"); fiberClass = defineClass("Fiber"); defineModule("FileTest"); diff --git a/src/main/java/org/truffleruby/core/array/ArrayNodes.java b/src/main/java/org/truffleruby/core/array/ArrayNodes.java index e3421087f302..ccbe0117a4b6 100644 --- a/src/main/java/org/truffleruby/core/array/ArrayNodes.java +++ b/src/main/java/org/truffleruby/core/array/ArrayNodes.java @@ -86,6 +86,7 @@ import org.truffleruby.language.library.RubyStringLibrary; import org.truffleruby.language.methods.Split; import org.truffleruby.language.objects.AllocationTracing; +import org.truffleruby.language.objects.IsANode; import org.truffleruby.language.objects.WriteObjectFieldNode; import org.truffleruby.language.objects.shared.IsSharedNode; import org.truffleruby.language.objects.shared.PropagateSharingNode; @@ -282,15 +283,23 @@ protected Object indexRange(RubyArray array, RubyRange range, NotProvided length return readSlice.executeReadSlice(array, startLength[0], len); } - @Specialization(guards = { "isRubyArithmeticSequence(index)" }) + @Specialization(guards = { "isANode.executeIsA(index, coreLibrary().arithmeticSequenceClass)" }) protected Object indexArithmeticSequence(RubyArray array, Object index, NotProvided length, - @Cached DispatchNode callSliceArithmeticSequence) { + @Cached IsANode isANode, + @Cached DispatchNode callSliceArithmeticSequence) { return callSliceArithmeticSequence.call(array, "slice_arithmetic_sequence", index); } @Specialization(guards = { "!isInteger(index)", "!isRubyRange(index)" }) protected Object indexFallback(RubyArray array, Object index, NotProvided length, + // @Cached IsANode isANode, @Cached AtNode accessWithIndexConversion) { + + // if (isANode.executeIsA(index, coreLibrary().arithmeticSequenceClass)) { + // CompilerDirectives.transferToInterpreterAndInvalidate(); + // return indexArithmeticSequence(array, index, length, isANode, DispatchNode.create()); + // } + return accessWithIndexConversion.executeAt(array, index); } diff --git a/src/main/java/org/truffleruby/language/RubyGuards.java b/src/main/java/org/truffleruby/language/RubyGuards.java index 2eabf982f0d6..8de7a212b3bf 100644 --- a/src/main/java/org/truffleruby/language/RubyGuards.java +++ b/src/main/java/org/truffleruby/language/RubyGuards.java @@ -169,10 +169,6 @@ public static boolean isNil(Object object) { return object == Nil.INSTANCE; } - public static boolean isRubyArithmeticSequence(Object object) { - return true; - } - // Internal types public static boolean isRubyDynamicObject(Object object) { From 73ac5e38ce25e1d46d3c4443276f4fa736e93f8b Mon Sep 17 00:00:00 2001 From: ccocchi Date: Mon, 1 Nov 2021 23:51:48 +0100 Subject: [PATCH 6/8] Manual fallback --- .../truffleruby/core/array/ArrayNodes.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/truffleruby/core/array/ArrayNodes.java b/src/main/java/org/truffleruby/core/array/ArrayNodes.java index ccbe0117a4b6..69abc9b16ef7 100644 --- a/src/main/java/org/truffleruby/core/array/ArrayNodes.java +++ b/src/main/java/org/truffleruby/core/array/ArrayNodes.java @@ -283,23 +283,21 @@ protected Object indexRange(RubyArray array, RubyRange range, NotProvided length return readSlice.executeReadSlice(array, startLength[0], len); } - @Specialization(guards = { "isANode.executeIsA(index, coreLibrary().arithmeticSequenceClass)" }) + @Specialization(guards = { "isArithmeticSequence(index, isANode)" }) protected Object indexArithmeticSequence(RubyArray array, Object index, NotProvided length, @Cached IsANode isANode, @Cached DispatchNode callSliceArithmeticSequence) { return callSliceArithmeticSequence.call(array, "slice_arithmetic_sequence", index); } - @Specialization(guards = { "!isInteger(index)", "!isRubyRange(index)" }) + @Specialization( + guards = { + "!isInteger(index)", + "!isRubyRange(index)", + "!isArithmeticSequence(index, isANode)" }) protected Object indexFallback(RubyArray array, Object index, NotProvided length, - // @Cached IsANode isANode, + @Cached IsANode isANode, @Cached AtNode accessWithIndexConversion) { - - // if (isANode.executeIsA(index, coreLibrary().arithmeticSequenceClass)) { - // CompilerDirectives.transferToInterpreterAndInvalidate(); - // return indexArithmeticSequence(array, index, length, isANode, DispatchNode.create()); - // } - return accessWithIndexConversion.executeAt(array, index); } @@ -322,6 +320,10 @@ protected Object sliceFallback(RubyArray array, Object start, Object length, @Cached ToIntNode lengthToInt) { return executeIntIndices(array, indexToInt.execute(start), lengthToInt.execute(length)); } + + protected boolean isArithmeticSequence(Object object, IsANode isANode) { + return isANode.executeIsA(object, coreLibrary().arithmeticSequenceClass); + } } @CoreMethod( From 235bc42679f7fed79d7425c214490a9ee6798252 Mon Sep 17 00:00:00 2001 From: ccocchi Date: Thu, 11 Nov 2021 21:34:40 +0100 Subject: [PATCH 7/8] Try to make it more readable --- src/main/ruby/truffleruby/core/array.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/ruby/truffleruby/core/array.rb b/src/main/ruby/truffleruby/core/array.rb index 4dc94a8001af..6290ce0b59a7 100644 --- a/src/main/ruby/truffleruby/core/array.rb +++ b/src/main/ruby/truffleruby/core/array.rb @@ -162,19 +162,21 @@ def ==(other) start ||= 0 # begin-less range stop ||= -1 # endless range - # negative indexes refer to end of array + # negative indexes refer to the end of array start += len if start < 0 stop += len if stop < 0 - if start < 0 || start > len - raise RangeError, "#{seq.inspect} out of range" if step < -1 || step > 1 - return nil - end - stop += 1 unless seq.exclude_end? diff = stop - start - raise RangeError, "#{seq.inspect} out of range" if diff > len && (step < -1 || step > 1) + is_out_of_bound = start < 0 || start > len + + if step < -1 || step > 1 + raise RangeError, "#{seq.inspect} out of range" if is_out_of_bound || diff > len + elsif is_out_of_bound + return nil + end + return [] if diff <= 0 diff = len - start if (len < diff || len < start + diff) From fc962c905d1c2b753c16699c7fce0ec1751c7332 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 18 Nov 2021 17:03:01 +0100 Subject: [PATCH 8/8] Add ChangeLog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aca3c5e52379..2b562e7ec357 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Compatibility: * Sort by default for `Dir.{glob,[]}` and add `sort:` keyword argument (#2523, @Strech). * Implement `rb_str_locktmp` and `rb_str_unlocktmp` (#2524, @bjfish). * Update `Kernel#instance_variables` to return insertion order (@bjfish). +* Implement `Array#slice` with `ArithmeticSequence` (#2526, @ccocchi). Performance: