https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112019-08-23T02:02:23ZRuby Issue Tracking SystemRuby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=809272019-08-23T02:02:23Zduerst (Martin Dürst)duerst@it.aoyama.ac.jp
<ul></ul><p>I was afraid that this would be an optimization for flat arrays, but increase time for nested arrays. But that's not the case, because at the bottom, there will be flat arrays, and flattening these will be faster.</p>
<p>However, I expect this to be slower on arrays that are 'almost flat', i.e.</p>
<pre><code>almost_flat_ary = 100.times.to_a + [1, 2]
</code></pre>
<p>Putting the nested array at the end will make sure all elements of the big array are checked, only to discover that actual flattening work is needed. The time needed for checking will not be offset by the allocation savings on the small array at the end.</p>
<p>Still I think that in general, this should be faster, and so it should be worth accepting this patch.</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=809322019-08-23T06:31:00Zshevegen (Robert A. Heiler)shevegen@gmail.com
<ul></ul><p>I use .flatten and .flatten! quite a lot in my ruby code, often setting the<br>
initial input from ARGV but also from other sources, such as used from other<br>
ruby classes, w hen I need to have an array - often something like:</p>
<pre><code>def foobar(input)
new_value = [input].flatten.compact
</code></pre>
<p>Or something like that. If this patch indeed improves this then that would<br>
be quite nice to have.</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=809332019-08-23T07:41:35ZHanmac (Hans Mackowiak)hanmac@gmx.de
<ul></ul><p>i see in your patch that you not only check for <code>Array</code> but for <code>to_ary</code> too, which is nice</p>
<p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/3414">@shevegen (Robert A. Heiler)</a> :</p>
<p>instead of <code>[input]</code> i would use <code>Array(input)</code> that doesn't create an extra array if input is already one</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=809382019-08-23T15:32:43Zdylants (Dylan Thacker-Smith)
<ul></ul><p>duerst (Martin Dürst) wrote:</p>
<blockquote>
<p>However, I expect this to be slower on arrays that are 'almost flat', i.e.</p>
<pre><code>almost_flat_ary = 100.times.to_a + [1, 2]
</code></pre>
<p>Putting the nested array at the end will make sure all elements of the big array are checked, only to discover that actual flattening work is needed. The time needed for checking will not be offset by the allocation savings on the small array at the end.</p>
</blockquote>
<p>The optimization handles this case by doing a quick <code>ary_memcpy</code> of everything up to the first nested array to avoid redundantly rechecking if those preceding elements were an array.</p>
<p>I benchmarked it by replacing <code>arrays</code> and <code>Benchmark.bmbm</code> in my above bechmark script with</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="n">ary</span> <span class="o">=</span> <span class="mi">100</span><span class="p">.</span><span class="nf">times</span><span class="p">.</span><span class="nf">to_a</span><span class="p">.</span><span class="nf">push</span><span class="p">([</span><span class="mi">101</span><span class="p">,</span> <span class="mi">102</span><span class="p">])</span>
<span class="no">Benchmark</span><span class="p">.</span><span class="nf">bmbm</span> <span class="k">do</span> <span class="o">|</span><span class="n">x</span><span class="o">|</span>
<span class="n">report</span><span class="p">(</span><span class="n">x</span><span class="p">,</span> <span class="s2">"flatten!"</span><span class="p">)</span> <span class="k">do</span>
<span class="n">ary</span><span class="p">.</span><span class="nf">flatten!</span>
<span class="k">end</span>
<span class="n">report</span><span class="p">(</span><span class="n">x</span><span class="p">,</span> <span class="s2">"flatten"</span><span class="p">)</span> <span class="k">do</span>
<span class="n">ary</span><span class="p">.</span><span class="nf">flatten</span>
<span class="k">end</span>
<span class="k">end</span>
</code></pre>
<p>and got the following results on master</p>
<pre><code> user system total real
flatten! 0.577976 0.002275 0.580251 ( 0.582700)
flatten 0.542454 0.001994 0.544448 ( 0.546523)
</code></pre>
<p>and the following results with this patch</p>
<pre><code> user system total real
flatten! 0.420922 0.001052 0.421974 ( 0.422957)
flatten 0.430728 0.001173 0.431901 ( 0.433274)
</code></pre>
<p>so I actually noticed improved performance for arrays with a large flat prefix.</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=809402019-08-23T15:38:06Zdylants (Dylan Thacker-Smith)
<ul></ul><p>Hanmac (Hans Mackowiak) wrote:</p>
<blockquote>
<p>i see in your patch that you not only check for <code>Array</code> but for <code>to_ary</code> too, which is nice</p>
</blockquote>
<p>Right, but that is just preserving the current behaviour. So the <code>ary.flatten! if ary.any?(Array)</code> workaround could actually be a breaking change if the given array had elements that responded to <code>to_ary</code> other than <code>Array</code>.</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=816032019-09-19T08:30:59Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>nobu (Nobuyoshi Nakada)</i></li></ul><p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/4">@nobu (Nobuyoshi Nakada)</a> Could you please review this?</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=816222019-09-20T00:19:48Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul></ul><p>This patch unrolls the <code>while</code>-loop for the already flatten head and seems reasonable.<br>
But I could get only insignificant result, and a worse result for an unflattened array.</p>
<pre><code class="yaml syntaxhl" data-language="yaml"><span class="c1"># array_flatten.yml</span>
<span class="na">prelude</span><span class="pi">:</span> <span class="pi">|</span>
<span class="s">ary = (0...100).to_a.push([101, 102])</span>
<span class="s">ary2 = 10.times.map {|i| (i..(i+9)).to_a}</span>
<span class="na">benchmark</span><span class="pi">:</span>
<span class="na">flatten!</span><span class="pi">:</span> <span class="s">ary.flatten!</span>
<span class="na">flatten</span><span class="pi">:</span> <span class="s">ary.flatten</span>
<span class="na">flatten2</span><span class="pi">:</span> <span class="s">ary2.flatten</span>
<span class="na">loop_count</span><span class="pi">:</span> <span class="m">1000000</span>
</code></pre>
<pre><code> benchmark/array_flatten.yml
Calculating -------------------------------------
compare-ruby built-ruby
flatten! 220.944k 223.612k i/s - 1.000M times in 4.526028s 4.472037s
flatten 216.457k 210.651k i/s - 1.000M times in 4.619859s 4.747182s
flatten2 182.447k 166.501k i/s - 1.000M times in 5.481056s 6.005954s
Comparison:
flatten!
built-ruby: 223611.7 i/s
compare-ruby: 220944.3 i/s - 1.01x slower
flatten
compare-ruby: 216456.8 i/s
built-ruby: 210651.3 i/s - 1.03x slower
flatten2
compare-ruby: 182446.6 i/s
built-ruby: 166501.4 i/s - 1.10x slower
</code></pre> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=816682019-09-22T18:29:53Zmethodmissing (Lourens Naudé)lourens@bearmetal.eu
<ul><li><strong>File</strong> <a href="/attachments/8046">0001-Let-empty-arrays-being-flattened-not-alloc-additiona.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/8046/0001-Let-empty-arrays-being-flattened-not-alloc-additiona.patch">0001-Let-empty-arrays-being-flattened-not-alloc-additiona.patch</a> added</li></ul><p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>This patch unrolls the <code>while</code>-loop for the already flatten head and seems reasonable.<br>
But I could get only insignificant result, and a worse result for an unflattened array.</p>
<pre><code class="yaml syntaxhl" data-language="yaml"><span class="c1"># array_flatten.yml</span>
<span class="na">prelude</span><span class="pi">:</span> <span class="pi">|</span>
<span class="s">ary = (0...100).to_a.push([101, 102])</span>
<span class="s">ary2 = 10.times.map {|i| (i..(i+9)).to_a}</span>
<span class="na">benchmark</span><span class="pi">:</span>
<span class="na">flatten!</span><span class="pi">:</span> <span class="s">ary.flatten!</span>
<span class="na">flatten</span><span class="pi">:</span> <span class="s">ary.flatten</span>
<span class="na">flatten2</span><span class="pi">:</span> <span class="s">ary2.flatten</span>
<span class="na">loop_count</span><span class="pi">:</span> <span class="m">1000000</span>
</code></pre>
<pre><code> benchmark/array_flatten.yml
Calculating -------------------------------------
compare-ruby built-ruby
flatten! 220.944k 223.612k i/s - 1.000M times in 4.526028s 4.472037s
flatten 216.457k 210.651k i/s - 1.000M times in 4.619859s 4.747182s
flatten2 182.447k 166.501k i/s - 1.000M times in 5.481056s 6.005954s
Comparison:
flatten!
built-ruby: 223611.7 i/s
compare-ruby: 220944.3 i/s - 1.01x slower
flatten
compare-ruby: 216456.8 i/s
built-ruby: 210651.3 i/s - 1.03x slower
flatten2
compare-ruby: 182446.6 i/s
built-ruby: 166501.4 i/s - 1.10x slower
</code></pre>
</blockquote>
<p>Attached is a patch for early return and preventing additional allocs for the empty array case (however not very sure how often that happens in practice):</p>
<pre><code>prelude: |
ary = []
benchmark:
flatten!: ary.flatten!
flatten: ary.flatten
loop_count: 1000000
</code></pre>
<pre><code>lourens@CarbonX1:~/src/ruby/ruby$ /usr/local/bin/ruby --disable=gems -rrubygems -I./benchmark/lib ./benchmark/benchmark-driver/exe/benchmark-driver --executables="compare-ruby::~/src/ruby/trunk/ruby --disable=gems -I.ext/common --disable-gem" --executables="built-ruby::./miniruby -I./lib -I. -I.ext/common -r./prelude --disable-gem" -v --repeat-count=24 $HOME/src/array_flatten.yml
compare-ruby: ruby 2.7.0dev (2019-09-22T17:21:54Z master 142efba93e) [x86_64-linux]
built-ruby: ruby 2.7.0dev (2019-09-22T18:10:18Z opt-flatten-empty-.. ae17889e1e) [x86_64-linux]
Calculating -------------------------------------
compare-ruby built-ruby
flatten! 6.234M 31.453M i/s - 1.000M times in 0.160418s 0.031794s
flatten 6.271M 31.324M i/s - 1.000M times in 0.159468s 0.031924s
Comparison:
flatten!
built-ruby: 31452589.1 i/s
compare-ruby: 6233726.0 i/s - 5.05x slower
flatten
built-ruby: 31324068.7 i/s
compare-ruby: 6270832.4 i/s - 5.00x slower
</code></pre> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=817582019-09-27T03:42:46Zdylants (Dylan Thacker-Smith)
<ul></ul><p>It looks like I made a mistake in my benchmarking of non-flattened arrays, since flatten! from the first iteration would cause further iterations to actually test with a flat array. It looks like the performance for arrays starting with a nested array are about the same with and without my patch.</p>
<p>The attached patch has a merge conflict. I've opened <a href="https://github.com/ruby/ruby/pull/2495" class="external">https://github.com/ruby/ruby/pull/2495</a> with a rebase of my patch that resolves the merge conflict. That PR also has an updated benchmark that uses the benchmark driver yaml format and includes the results from running that benchmark.</p>
<p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>But I could get only insignificant result, and a worse result for an unflattened array.</p>
</blockquote>
<p>It looks like nobu's benchmark/array_flatten.yml has the same problem with the non-flat arrays being mutated and affecting following iterations. That explains the improvement I am seeing locally with your benchmark, although I don't know why my results differ from what you posted</p>
<pre><code>Comparison:
flatten!
built-ruby: 246214.2 i/s
compare-ruby: 193998.4 i/s - 1.27x slower
flatten
built-ruby: 214275.0 i/s
compare-ruby: 188818.6 i/s - 1.13x slower
flatten2
built-ruby: 159840.8 i/s
compare-ruby: 159546.4 i/s - 1.00x slower
</code></pre>
<p>where only the last one actually benchmarks an unflattened array properly.</p>
<p>methodmissing (Lourens Naudé) wrote:</p>
<blockquote>
<p>Attached is a patch for early return and preventing additional allocs for the empty array case (however not very sure how often that happens in practice):</p>
</blockquote>
<p>There is no need to special case the empty array case. My patch would just treat it as a flat array, so would still avoid extra allocations and return early.</p> Ruby master - Feature #16119: Optimize Array#flatten and flatten! for already flattened arrayshttps://bugs.ruby-lang.org/issues/16119?journal_id=817742019-09-27T16:24:53Zdylants (Dylan Thacker-Smith)
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Applied in changeset <a class="changeset" title="Optimize Array#flatten and flatten! for already flattened arrays (#2495) * Optimize Array#flatte..." href="https://bugs.ruby-lang.org/projects/ruby-master/repository/git/revisions/a1fda16b238f24cf55814ecc18f716cbfff8dd91">git|a1fda16b238f24cf55814ecc18f716cbfff8dd91</a>.</p>
<hr>
<p>Optimize Array#flatten and flatten! for already flattened arrays (<a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Matrix: Vector#each2 should check its argument (Closed)" href="https://bugs.ruby-lang.org/issues/2495">#2495</a>)</p>
<ul>
<li>Optimize Array#flatten and flatten! for already flattened arrays</li>
<li>Add benchmark for Array#flatten and Array#flatten!</li>
</ul>
<p>[Bug <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Optimize Array#flatten and flatten! for already flattened arrays (Closed)" href="https://bugs.ruby-lang.org/issues/16119">#16119</a>]</p>