Use Map functionality for into and reduce

[TylerWitt]

There are two cases where Enum functionality is used over Map functionality.

1. Enum.reduce, when the acc is only used for Map.put(acc, ...) -- in this case, Map.new is equivalent, and skips :lists.foldl in favor of Enum.map |> :maps.from_list, which is faster based on benchmarks by almost 1.5x.

2. Enum.into(enum, %{}) -- in this case, Map.new is equivalent, but is more readable.

Mix.install([{:benchee, "~> 1.0"}])

enum = Enum.to_list(1..10000)

Benchee.run(
  %{
    "map_new" => fn -> Map.new(enum, &{&1, &1}) end,
    "enum_into" => fn -> Enum.into(enum, %{}, &{&1, &1}) end,
    "enum_reduce" => fn -> Enum.reduce(enum, %{}, &Map.put(&2, &1, &1)) end
  },
  time: 10,
  memory_time: 2
)

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 10 s
memory time: 2 s
reduction time: 0 ns
parallel: 1
inputs: none specified
Estimated total run time: 42 s

Benchmarking enum_into ...
Benchmarking enum_reduce ...
Benchmarking map_new ...
Calculating statistics...
Formatting results...

Name                  ips        average  deviation         median         99th %
map_new            538.35        1.86 ms    ±14.97%        1.75 ms        3.09 ms
enum_into          522.59        1.91 ms    ±30.75%        1.77 ms        3.24 ms
enum_reduce        365.72        2.73 ms    ±36.09%        2.51 ms        4.64 ms

Comparison: 
map_new            538.35
enum_into          522.59 - 1.03x slower +0.0560 ms
enum_reduce        365.72 - 1.47x slower +0.88 ms

Memory usage statistics:

Name           Memory usage
map_new           430.39 KB
enum_into         430.39 KB - 1.00x memory usage +0 KB
enum_reduce      1709.70 KB - 3.97x memory usage +1279.30 KB

**All measurements for memory usage were the same**

[binaryseed]

The benchmark here is not really representative of the behavior we'd see in a real app. None of these functions would ever get called with 10_000 items, generally it'll be single digits, probably maxing out at 100. Re-running the benchmark shows that with a list of 100 items, we're talking about ~10 microsecond difference.

I think avoiding reduce + put is a good change though, but I'm going to commit some other changes before I merge.

Generally, while I'm grateful for your interest, I'm disinclined to accept these kinds of minor tweaks in the future, especially changes don't impact behavior or performance in a meaningful way.

[TylerWitt]

Generally, while I'm grateful for your interest, I'm disinclined to accept these kinds of minor tweaks in the future, especially changes don't impact behavior or performance in a meaningful way.

The Enum.reduce memory usage surprised me, to be fair!

I totally understand, though I do feel like over time these kinds of changes do end up making a difference.

Are there any changes you have in mind that you would accept? I have some ideas that would make the tests simpler, but I'm not sure you would be okay with it, and I don't want to spend the time doing it if that's the case.

[binaryseed]

This library is complex and specialized, and is relied upon by lots of applications. It has been stable for a long time, and while we have been adding new features recently, less change is better in the long run.

You could open an Issue and make a quick description of your proposed changes, but don't be disappointed if we turn them down ideas that introduce a lot of churn, touch sensitive areas, or make changes for subjective reasons.