O(N) Array#delete_if

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<T> *vector, size_t index)

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

src/array_object.cpp

@@ -692,17 +692,37 @@ Value ArrayObject::delete_if(Env *env, Block *block) {
 
     Vector<size_t> 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()) {
             marked_indexes.push(i);
         }
     }
 
-    while (!marked_indexes.is_empty()) {
-        m_vector.remove(marked_indexes.pop());
-    }
-
     return this;
 }
 

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