Optimize lists:flatmap/2 for filtering

[sabiwara]

In case there is an interest, I'm proposing this optimization we added to elixir (elixir-lang/elixir#13793), since the implementation and the expected improvement are the same. Feel free to close if you feel it is unnecessary.

This could especially generate important speedups (1.5~3x) for cases where flatmap is used as a filtermap, like this or this, but should benefit any case which returns some amount of empty lists or size-1 lists.

[github-actions]

CT Test Results

No tests were run for this PR. This is either because the build failed, or the PR is based on a branch without GH actions tests configured.

Results for commit 29bb2b4

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[bjorng]

I think this is a worthwhile optimization.

I only have one minor comment; please amend the commit and force-push when you do the correction.

[Maria-12648430]

@bjorng @sabiwara could you explain to me why this change actually results in such a considerable (1.5-3x) speedup? The optimization cases, namely appending to an empty list vs passing through and appending to a singleton list vs consing the singleton, don't seem to explain this.

[jhogberg]

BIF call overhead is considerable compared to consing or doing nothing at all.

[bjorng]

I did some measurements using your version of append/1 compared to lists:append/1:

% erlperf --init_runner_all 'lists:duplicate(10_000, []).' 'r(L) -> lists:append(L).' 'r(L) -> t:append(L).'
Code                             ||        QPS       Time   Rel
r(L) -> t:append(L).              1      58535   17089 ns  100%
r(L) -> lists:append(L).          1       8179     122 us   14%
% erlperf --init_runner_all 'lists:duplicate(10_000, [x]).' 'r(L) -> lists:append(L).' 'r(L) -> t:append(L).'
Code                             ||        QPS       Time   Rel
r(L) -> t:append(L).              1      11444   87408 ns  100%
r(L) -> lists:append(L).          1       5311     188 us   46%
% erlperf --init_runner_all 'lists:duplicate(10_000, [x,y,z]).' 'r(L) -> lists:append(L).' 'r(L) -> t:append(L).'
Code                             ||        QPS       Time   Rel
r(L) -> lists:append(L).          1       3789     264 us  100%
r(L) -> t:append(L).              1       3716     269 us   98%

According to these measurements, your version is faster if all lists have 0 or 1 elements. As soon as most lists have more than one element, you will lose time by having those extra clauses.

[Maria-12648430]

Ah, I get it now 💡 The performance gain is most pronounced when the given Fun returns empty or singleton lists, and diminishes when it returns longer lists.

[Maria-12648430]

@bjorng you beat me to it XD

[Maria-12648430]

@bjorng that said... doesn't that mean that with the changes in this PR, lists:flatmap also got slower when the Fun returns lists with more than 1 element? 😅 That is, doesn't it thereby improve a special case (flatmap being used as filter/map/filtermap) at the expense of the general case?

[bjorng]

@Maria-12648430

doesn't that mean that with the changes in this PR, lists:flatmap also got slower when the Fun returns lists with more than 1 element? 😅

Yes, it does. I think it's worth it because using flatmap for filtering is a common use case, and it only gets a little bit slower when the lists have more than one element. Another way to express that is that the gain for the filtering use case is much higher than the small loss for the general use case.