diff --git a/ext/tm/include/tm/vector.hpp b/ext/tm/include/tm/vector.hpp index 05964b1896..102ccac891 100644 --- a/ext/tm/include/tm/vector.hpp +++ b/ext/tm/include/tm/vector.hpp @@ -752,6 +752,14 @@ class Vector { grow_at_least(new_size); } + /** + * Shrink the size of the vector. + */ + void resize(size_t new_size) { + assert(new_size <= m_size); + m_size = new_size; + } + class iterator { public: iterator(const Vector *vector, size_t index) diff --git a/spec/core/array/delete_if_spec.rb b/spec/core/array/delete_if_spec.rb index 1459cc8d04..10972eee0e 100644 --- a/spec/core/array/delete_if_spec.rb +++ b/spec/core/array/delete_if_spec.rb @@ -2,6 +2,7 @@ require_relative 'fixtures/classes' require_relative 'shared/enumeratorize' require_relative 'shared/delete_if' +require_relative 'shared/iterable_and_tolerating_size_increasing' require_relative '../enumerable/shared/enumeratorized' describe "Array#delete_if" do @@ -47,6 +48,35 @@ -> { ArraySpecs.empty_frozen_array.delete_if {} }.should raise_error(FrozenError) end + it "does not truncate the array is the block raises an exception" do + a = [1, 2, 3] + begin + a.delete_if { raise StandardError, 'Oops' } + rescue + end + + a.should == [1, 2, 3] + end + + it "only removes elements for which the block returns true, keeping the element which raised an error." do + a = [1, 2, 3, 4] + begin + a.delete_if do |e| + case e + when 2 then true + when 3 then raise StandardError, 'Oops' + else false + end + end + rescue StandardError + end + + a.should == [1, 3, 4] + end + it_behaves_like :enumeratorized_with_origin_size, :delete_if, [1,2,3] it_behaves_like :delete_if, :delete_if + + @value_to_return = -> _ { false } + it_behaves_like :array_iterable_and_tolerating_size_increasing, :delete_if end diff --git a/src/array_object.cpp b/src/array_object.cpp index 68274f5d6c..32b96a6f3e 100644 --- a/src/array_object.cpp +++ b/src/array_object.cpp @@ -692,6 +692,30 @@ Value ArrayObject::delete_if(Env *env, Block *block) { Vector marked_indexes; + // O(N) + Defer remove_marked_indexes([&]() { + if (marked_indexes.size() > 0) { + auto now = time(NULL); + printf("Starting to remove at %s...\n", ctime(&now)); + marked_indexes.push(m_vector.size()); + size_t from = 0, self_index = 0; + for (size_t limit: marked_indexes) { + size_t n_to_move = limit - from; + // std::move not supported by TM::vector iterator :( + for (size_t i = 0; i < n_to_move; i++) + m_vector[self_index + i] = m_vector[from + i]; + self_index += n_to_move; + from = limit + 1; + } + m_vector.resize(m_vector.size() - marked_indexes.size() + 1); + now = time(NULL); + printf("...finished removing at %s\n", ctime(&now)); + } + }); + + auto now = time(NULL); + printf("\n#delete_if actually starting at %s...\n", ctime(&now)); + for (size_t i = 0; i < size(); ++i) { Value result = block->run(env, { (*this)[i] }, nullptr); if (result.is_truthy()) { @@ -699,10 +723,6 @@ Value ArrayObject::delete_if(Env *env, Block *block) { } } - while (!marked_indexes.is_empty()) { - m_vector.remove(marked_indexes.pop()); - } - return this; } diff --git a/test/natalie/array_test.rb b/test/natalie/array_test.rb index fab0ea6fad..ba520689d1 100644 --- a/test/natalie/array_test.rb +++ b/test/natalie/array_test.rb @@ -2045,4 +2045,11 @@ def initialize a.object_id.should == id_was end end + + describe '#delete_if' do + it 'should be O(N)' do + # fake test just to see if it times out + (1..100000000).to_a.delete_if { |i| i % 10 == 0 }.count.should == 90000000 + end + end end