[GR-19220] Improvements to Array#sample (#2148)

PullRequest: truffleruby/2155

Author: eregon

.rubocop.yml

@@ -179,10 +179,8 @@ Lint/InheritException:
 
 Lint/LiteralAsCondition:
   Description: Checks of literals used in conditions.
-  Details: >-
-      Pattern `while true` is allowed, disable the cop with
-      `while true # rubocop:disable Lint/LiteralAsCondition` in this case.
-  Enabled: true
+  # disabled, it prevents `while true`, which is useful
+  Enabled: false
 
 # Supports --auto-correct
 Lint/LiteralInInterpolation:

src/main/ruby/truffleruby/core/array.rb

@@ -975,7 +975,7 @@ def sample(count=undefined, options=undefined)
     end
 
     if count and count < 0
-      raise ArgumentError, 'count must be greater than 0'
+      raise ArgumentError, 'count must be >= 0'
     end
 
     rng = options[:random] if options
@@ -996,80 +996,119 @@ def sample(count=undefined, options=undefined)
       return [at(rng.rand(size))]
     when 2
       i = rng.rand(size)
-      j = rng.rand(size)
-      if i == j
-        j = i == 0 ? i + 1 : i - 1
+      j = rng.rand(size - 1)
+      if j >= i
+        j += 1
       end
       return [at(i), at(j)]
     else
       sample_many(count, rng)
     end
   end
 
-  def sample_many(count, rng)
-    if size / count > 3
-      abandon = false
+  private def sample_many(count, rng)
+    if count <= 70 # three implementations; choice determined experimentally
+      if 2.0 * size / count  <= count  + 13
+        sample_many_swap(count, rng)
+      else
+        sample_many_quad(count, rng)
+      end
+    else
+      if size <= -1100.0 + 59.5 * count
+        sample_many_swap(count, rng)
+      else
+        sample_many_hash(count,rng)
+      end
+    end
+  end
 
-      result = Array.new count
-      i = 1
+  private def sample_many_swap(count, rng)
+    # linear dependence on array size, therefore very slow for small count / size
+    result = Array.new(self)
 
-      result[0] = rng.rand(size)
-      while i < count
-        k = rng.rand(size)
+    count.times do |c|
+      result.__send__ :swap, c, rng.rand(size)
+    end
 
-        spin = false
-        spin_count = 0
+    count == size ? result : result[0, count]
+  end
 
-        while true # rubocop:disable Lint/LiteralAsCondition
-          j = 0
-          while j < i
-            if k == result[j]
-              spin = true
-              break
-            end
+  private def sample_many_quad(count, rng)
+    # quadratic time due to linear time collision check but low overhead
+    result = Array.new count
+    i = 1
 
-            j += 1
-          end
+    result[0] = rng.rand(size)
 
-          if spin
-            if (spin_count += 1) > 100
-              abandon = true
-              break
-            end
+    while i < count
+      k = rng.rand(size)
+      spin = false
 
-            k = rng.rand(size)
-          else
+      while true
+        j = 0
+        while j < i
+          if k == result[j]
+            spin = true
             break
           end
+
+          j += 1
         end
 
-        break if abandon
+        if spin
+          k = rng.rand(size)
+          spin = false
+        else
+          break
+        end
+      end
 
-        result[i] = k
+      result[i] = k
+      i += 1
+    end
 
-        i += 1
-      end
+    i = 0
+    while i < count
+      result[i] = at result[i]
+      i += 1
+    end
 
-      unless abandon
-        i = 0
-        while i < count
-          result[i] = at result[i]
-          i += 1
-        end
+    result
+  end
+
+  private def sample_many_hash(count, rng)
+    # use hash for constant time collision check but higher overhead
+    result = Array.new count
+    i = 1
 
-        return result
+    result[0] = rng.rand(size)
+    result_set = { result[0] => 0 }
+
+    while i < count
+      k = rng.rand(size)
+
+      while true
+        if result_set.include?(k)
+          k = rng.rand(size)
+        else
+          break
+        end
       end
-    end
 
-    result = Array.new(self)
+      result[i] = k
+      result_set[i] = k
 
-    count.times do |c|
-      result.__send__ :swap, c, rng.rand(size)
+      i += 1
     end
 
-    count == size ? result : result[0, count]
+    i = 0
+    while i < count
+      result[i] = at result[i]
+      i += 1
+    end
+
+    result
   end
-  private :sample_many
 
   def select!(&block)
     return to_enum(:select!) { size } unless block_given?
@@ -1079,7 +1118,6 @@ def select!(&block)
     ary = select(&block)
     Primitive.steal_array_storage(self, ary) unless size == ary.size
   end
-
   alias_method :filter!, :select!
 
   def shuffle(options = undefined)
@@ -1188,7 +1226,6 @@ def unshift(*values)
 
     self
   end
-
   alias_method :prepend, :unshift
 
   def values_at(*args)

src/main/ruby/truffleruby/core/enumerable.rb

@@ -553,7 +553,7 @@ def cycle(many=nil)
         i += 1
       end
     else
-      while true # rubocop:disable Lint/LiteralAsCondition
+      while true
         cache.each { |o| yield o }
       end
     end

src/main/ruby/truffleruby/core/kernel.rb

@@ -434,7 +434,7 @@ def loop
     return to_enum(:loop) { Float::INFINITY } unless block_given?
 
     begin
-      while true # rubocop:disable Lint/LiteralAsCondition
+      while true
         yield
       end
     rescue StopIteration => si

src/main/ruby/truffleruby/core/posix.rb

@@ -346,7 +346,7 @@ def self.with_array_of_strings_pointer(strings)
   # gets as soon as it gets something.
 
   def self.read_string_at_least_one_byte(io, count)
-    while true # rubocop:disable Lint/LiteralAsCondition
+    while true
       # must call #read_string in order to properly support polyglot STDIO
       string, errno = read_string(io, count)
       return string if errno == 0

src/main/ruby/truffleruby/core/range.rb

@@ -26,8 +26,6 @@
 # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-# rubocop:disable Lint/LiteralAsCondition
-
 class Range
 
   include Enumerable