(PUP-11755) Avoid slow, buggy pattern for enum functions

The each/filter/map functions, for some dispatches, use patterns that
are slow and/or buggy when run on jruby.

The correct pattern for enumeration is

```
object.each do |value|
  yield(value)
end
```

Two alternatives in use are

```
enum = object.each
object.size.times do
  yield(enum.next)
end

enum = object.each

begin
  loop do
    yield(enum.next)
  end
rescue StopIteration
end
```

The first pattern is incorrect because it never triggers StopIteration
on the enumerator (by attempting to call `next` one last time). On MRI
this is apparently harmless, but on JRuby it leaves the Fiber associated with the
Enumerator around. Since Fibers are implemented via native threads on
JRuby, that leaves around a native thread. That can deplete the
available threads and interferes with garbage collection, as every
native thread functions as a GC root (leaving objects referenced but
functionally inaccessible).

The second pattern is technically correct but extremely inefficient on
JRuby, presumably due to the overhead associated with using a Fiber for
enumeration. It's also no more correct than simply calling the enum
method directly, so we can simply change it.

In benchmarks, the difference on MRI is ~10x and on JRuby it's ~150x.

Author: nicklewis

lib/puppet/functions/each.rb

@@ -116,10 +116,9 @@
   end
 
   def foreach_Hash_1(hash)
-    enumerator = hash.each_pair
     begin
-      hash.size.times do
-        yield(enumerator.next)
+      hash.each_pair do |pair|
+        yield(pair)
       end
     rescue StopIteration
     end
@@ -128,10 +127,9 @@ def foreach_Hash_1(hash)
   end
 
   def foreach_Hash_2(hash)
-    enumerator = hash.each_pair
     begin
-      hash.size.times do
-        yield(*enumerator.next)
+      hash.each_pair do |pair|
+        yield(*pair)
       end
     rescue StopIteration
     end
@@ -141,10 +139,12 @@ def foreach_Hash_2(hash)
 
   def foreach_Enumerable_1(enumerable)
     enum = Puppet::Pops::Types::Iterable.asserted_iterable(self, enumerable)
-      begin
-        loop { yield(enum.next) }
-      rescue StopIteration
+    begin
+      enum.each do |value|
+        yield value
       end
+    rescue StopIteration
+    end
     # produces the receiver
     enumerable
   end
@@ -155,10 +155,8 @@ def foreach_Enumerable_2(enumerable)
       enum.each { |entry| yield(*entry) }
     else
       begin
-        index = 0
-        loop do
-          yield(index, enum.next)
-          index += 1
+        enum.each_with_index do |value, index|
+          yield(index, value)
         end
       rescue StopIteration
       end

lib/puppet/functions/filter.rb

@@ -109,11 +109,8 @@ def filter_Enumerable_1(enumerable)
     result = []
     enum = Puppet::Pops::Types::Iterable.asserted_iterable(self, enumerable)
     begin
-      loop do
-        it = enum.next
-        if yield(it)
-          result << it
-        end
+      enum.each do |value|
+        result << value if yield(value)
       end
     rescue StopIteration
     end
@@ -124,18 +121,13 @@ def filter_Enumerable_2(enumerable)
     enum = Puppet::Pops::Types::Iterable.asserted_iterable(self, enumerable)
     if enum.hash_style?
       result = {}
-      enum.each { |k, v| result[k] = v if yield(k, v) }
+      enum.each {| k, v| result[k] = v if yield(k, v) }
       result
     else
       result = []
       begin
-        index = 0
-        loop do
-          it = enum.next
-          if yield(index, it)
-            result << it
-          end
-          index += 1
+        enum.each_with_index do |value, index|
+          result << value if yield(index, value)
         end
       rescue StopIteration
       end

lib/puppet/functions/map.rb

@@ -90,7 +90,7 @@
   def map_Hash_1(hash)
     result = []
     begin
-      hash.map {|x, y| result << yield([x, y]) }
+      hash.each {|x, y| result << yield([x, y]) }
     rescue StopIteration
     end
     result
@@ -99,7 +99,7 @@ def map_Hash_1(hash)
   def map_Hash_2(hash)
     result = []
     begin
-      hash.map {|x, y| result << yield(x, y) }
+      hash.each {|x, y| result << yield(x, y) }
     rescue StopIteration
     end
     result
@@ -109,7 +109,9 @@ def map_Enumerable_1(enumerable)
     result = []
     enum = Puppet::Pops::Types::Iterable.asserted_iterable(self, enumerable)
     begin
-      loop { result << yield(enum.next) }
+      enum.each do |val|
+        result << yield(val)
+      end
     rescue StopIteration
     end
     result
@@ -122,14 +124,11 @@ def map_Enumerable_2(enumerable)
     else
       result = []
       begin
-        index = 0
-        loop do
-          result << yield(index, enum.next)
-          index = index + 1
+        enum.each_with_index do |val, index|
+          result << yield(index, val)
         end
       rescue StopIteration
       end
-
       result
     end
   end

lib/puppet/pops/lookup/context.rb

@@ -92,12 +92,11 @@ def cached_value(key)
 
   def cached_entries(&block)
     if block_given?
-      enumerator = @cache.each_pair
-      @cache.size.times do
+      @cache.each_pair do |pair|
         if block.arity == 2
-          yield(*enumerator.next)
+          yield(*pair)
         else
-          yield(enumerator.next)
+          yield(pair)
         end
       end
       nil