perf(common): remove recursion when flattening collections

[cpcloud]

Follow-up/alternative to #8709.

[kszucs]

I also tried this approach but actually got worse results. Did you run the benchmarks?

A little bit more context here: in the majority of the cases there are either no sequence/mapping arguments or just a single one, so usually these recursions are 1-2 level deep.

[cpcloud]

The benchmarks look nearly exactly the same to me in both cases:

flatten-collections, recursive implementation:

---------------------------------------------------------------------------- benchmark 'test_bfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_bfs (0001_b2a4fbb)     6.3162 (1.04)     6.6793 (1.07)     6.3851 (1.04)     0.0736 (1.85)     6.3668 (1.04)     0.0538 (1.65)          8;6  156.6140 (0.96)         71           1
test_bfs (NOW)              6.0468 (1.0)      6.2498 (1.0)      6.1167 (1.0)      0.0397 (1.0)      6.1077 (1.0)      0.0327 (1.0)          17;6  163.4860 (1.0)          70           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_dfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_dfs (0001_b2a4fbb)     6.4195 (1.09)     6.6455 (1.10)     6.4510 (1.09)     0.0317 (1.0)      6.4401 (1.09)     0.0241 (1.0)           6;4  155.0149 (0.92)         73           1
test_dfs (NOW)              5.8935 (1.0)      6.0624 (1.0)      5.9405 (1.0)      0.0380 (1.20)     5.9327 (1.0)      0.0300 (1.25)         15;7  168.3354 (1.0)          74           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------ benchmark 'test_replace_mapping': 2 tests ------------------------------------------------------------------------------
Name (time in ms)                           Min                Max               Mean            StdDev             Median               IQR            Outliers      OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_mapping (0001_b2a4fbb)     24.8894 (1.0)      58.2770 (1.0)      26.1179 (1.0)      5.9696 (1.0)      25.0164 (1.0)      0.1337 (1.0)           1;4  38.2878 (1.0)          31           1
test_replace_mapping (NOW)              24.9994 (1.00)     59.8741 (1.03)     26.8665 (1.03)     6.1376 (1.03)     25.9590 (1.04)     0.5752 (4.30)          1;1  37.2211 (0.97)         31           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------ benchmark 'test_replace_pattern': 2 tests ------------------------------------------------------------------------------
Name (time in ms)                           Min                Max               Mean            StdDev             Median               IQR            Outliers      OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_pattern (0001_b2a4fbb)     35.8452 (1.0)      36.6991 (1.0)      36.3845 (1.0)      0.3027 (1.0)      36.5231 (1.00)     0.4296 (1.0)           4;0  27.4842 (1.0)          14           1
test_replace_pattern (NOW)              36.0548 (1.01)     69.4589 (1.89)     37.8864 (1.04)     6.8888 (22.76)    36.5112 (1.0)      0.4867 (1.13)          1;1  26.3947 (0.96)         23           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

flatten-collections-no-recursion, iterative implementation:

---------------------------------------------------------------------------- benchmark 'test_bfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_bfs (0001_b2a4fbb)     6.5196 (1.07)     6.8486 (1.05)     6.6194 (1.08)     0.0661 (1.22)     6.6486 (1.09)     0.1090 (5.19)         27;1  151.0721 (0.92)         68           1
test_bfs (NOW)              6.0766 (1.0)      6.5159 (1.0)      6.1202 (1.0)      0.0544 (1.0)      6.1103 (1.0)      0.0210 (1.0)           4;5  163.3942 (1.0)          66           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_dfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_dfs (0001_b2a4fbb)     6.6406 (1.07)     6.7605 (1.07)     6.6709 (1.07)     0.0160 (1.0)      6.6691 (1.07)     0.0112 (1.0)          10;4  149.9055 (0.94)         71           1
test_dfs (NOW)              6.2221 (1.0)      6.3236 (1.0)      6.2494 (1.0)      0.0173 (1.08)     6.2464 (1.0)      0.0231 (2.07)         19;1  160.0148 (1.0)          72           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------ benchmark 'test_replace_mapping': 2 tests ------------------------------------------------------------------------------
Name (time in ms)                           Min                Max               Mean            StdDev             Median               IQR            Outliers      OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_mapping (0001_b2a4fbb)     25.6490 (1.01)     59.8721 (1.00)     26.8985 (1.01)     6.2281 (1.00)     25.7645 (1.01)     0.0958 (1.19)          1;1  37.1768 (0.99)         30           1
test_replace_mapping (NOW)              25.4951 (1.0)      59.5866 (1.0)      26.7478 (1.0)      6.2027 (1.0)      25.6110 (1.0)      0.0802 (1.0)           1;2  37.3863 (1.0)          30           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------ benchmark 'test_replace_pattern': 2 tests ------------------------------------------------------------------------------
Name (time in ms)                           Min                Max               Mean            StdDev             Median               IQR            Outliers      OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_pattern (0001_b2a4fbb)     36.8889 (1.01)     37.4855 (1.02)     37.0912 (1.01)     0.1859 (2.22)     37.0316 (1.01)     0.2413 (1.70)          4;0  26.9606 (0.99)         13           1
test_replace_pattern (NOW)              36.5852 (1.0)      36.8580 (1.0)      36.6935 (1.0)      0.0837 (1.0)      36.6849 (1.0)      0.1419 (1.0)           3;0  27.2528 (1.0)          13           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Both approaches are an improvement, and I definitely prefer the iterative version for two reasons:

1. There's effectively no contest for deeply nested collections, iteration will win every time in a language that doesn't implement tail recursion. We simply don't have to think about the recursion limit anymore when calling this function. While we may never hit it, that seems like a weak justification for preferring recursion given the difference in complexity between implementations is very little.
2. The iterative approach likely uses a lot less memory, because it's not storing entire stack frames for every nesting level.

[kszucs]

I compared the two directly (NOW is the recursive, the other is the iterative):

---------------------------------------------------------------------------- benchmark 'test_bfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_bfs (0688_cd22d60)     1.9456 (1.05)     2.4202 (1.01)     2.0163 (1.05)     0.0809 (1.27)     1.9836 (1.05)     0.0797 (1.21)        23;10  495.9603 (0.95)        181           1
test_bfs (NOW)              1.8557 (1.0)      2.3946 (1.0)      1.9128 (1.0)      0.0635 (1.0)      1.8925 (1.0)      0.0660 (1.0)          23;7  522.7808 (1.0)         197           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_dfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_dfs (0688_cd22d60)     1.9546 (1.06)     2.5424 (1.12)     2.0125 (1.04)     0.0626 (1.0)      1.9991 (1.05)     0.0126 (1.0)         14;69  496.8840 (0.96)        202           1
test_dfs (NOW)              1.8480 (1.0)      2.2640 (1.0)      1.9277 (1.0)      0.0734 (1.17)     1.9030 (1.0)      0.0854 (6.78)         31;7  518.7529 (1.0)         196           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

----------------------------------------------------------------------------- benchmark 'test_replace_mapping': 2 tests -----------------------------------------------------------------------------
Name (time in ms)                          Min                Max               Mean            StdDev            Median               IQR            Outliers      OPS            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_mapping (0688_cd22d60)     8.6472 (1.01)     31.1602 (1.04)     10.0228 (1.00)     3.7096 (1.02)     9.3555 (1.00)     0.4533 (1.40)          3;4  99.7721 (1.00)         84           1
test_replace_mapping (NOW)              8.5944 (1.0)      29.8957 (1.0)      10.0029 (1.0)      3.6465 (1.0)      9.3373 (1.0)      0.3230 (1.0)           3;4  99.9708 (1.0)          85           1
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------ benchmark 'test_replace_pattern': 2 tests ------------------------------------------------------------------------------
Name (time in ms)                           Min                Max               Mean            StdDev             Median               IQR            Outliers      OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_pattern (0688_cd22d60)     12.2349 (1.0)      30.9696 (1.0)      13.4811 (1.01)     3.0946 (1.0)      12.8625 (1.01)     0.9684 (1.65)          2;2  74.1781 (0.99)         63           1
test_replace_pattern (NOW)              12.2737 (1.00)     32.9309 (1.06)     13.3990 (1.0)      3.4491 (1.11)     12.7602 (1.0)      0.5868 (1.0)           2;2  74.6326 (1.0)          61           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

For the record, I'm in favor of the iterative approach as well.

[cpcloud]

Definitely ready to concede this one.

The level of recursion here appears to be something that is internal and controlled by Ibis, i.e., there's no obvious way that a user can construct arbitrarily deep sequences that flattened by this function and would thus potentially hit Python's recursion limit.