Tweak unionWithKey.goDifferentHash

[sjakobi]

I discovered this one while copying the code for merge (#226).

Not sure whether it's actually correct or useful, but tests have passed so far.

t1 and t2 are always either Leaf or Collision, in case that matters.

---

TODO:

• Check effects on Core
• Benchmark

[treeowl]

Yeah, looks good to me!

[treeowl]

You going to unWIP this? Or still considering?

[sjakobi]

You going to unWIP this? Or still considering?

Yeah, once I've checked the Core and run the benchmarks. Feel free to take over if you're interested though.

[sjakobi]

I've had a look at the Core generated with GHC 8.10.7. The most significant effect is that in the Strict version, the goDifferentHash worker is floated out. Previously that was probably prevented because it was mutually recursive with go.

Also its BitmapIndexed return value is unboxed now, which seems beneficial.

The Lazy version doesn't have any notable changes AFAICT.

See for yourself:

This branch:

• Internal.dump-simpl.txt
• Strict.dump-simpl.txt

master:

• Internal.dump-simpl.txt
• Strict.dump-simpl.txt

---

EDIT: Steps for my own future reference:

• Add -ddump-simpl -dsuppress-all -dsuppress-uniques -ddump-to-file to ghc-options
• cabal clean; cabal build
• mkdir _branch _master
• cp -r dist-newstyle/build/x86_64-linux/ghc-8.10.7/unordered-containers-0.2.15.0/build/Data _branch/
• Same steps on master
• meld _master _branch

[sjakobi]

In the HashMap/union benchmark I'm seeing a speedup of around 1% which seems significant:

master (2 runs):

benchmarked HashMap/union
time                 69.16 μs   (69.00 μs .. 69.35 μs)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 69.29 μs   (69.23 μs .. 69.35 μs)
std dev              198.0 ns   (155.3 ns .. 259.6 ns)

benchmarked HashMap/union
time                 69.05 μs   (68.67 μs .. 69.42 μs)
                     1.000 R²   (0.999 R² .. 1.000 R²)
mean                 68.96 μs   (68.81 μs .. 69.24 μs)
std dev              598.9 ns   (293.3 ns .. 946.4 ns)

This branch (also 2 runs):

benchmarked HashMap/union
time                 68.24 μs   (68.10 μs .. 68.42 μs)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 68.26 μs   (68.22 μs .. 68.32 μs)
std dev              154.3 ns   (107.2 ns .. 204.9 ns)

benchmarked HashMap/union
time                 68.10 μs   (68.05 μs .. 68.15 μs)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 68.46 μs   (68.39 μs .. 68.54 μs)
std dev              249.1 ns   (198.1 ns .. 304.8 ns)

Unfortunately we don't have any benchmarks for the strict union variants, where I'd expect a larger impact.

Nevertheless it seems to me that all signs point in the right direction, so I'm inclined to merge.