Use tree-diffing for difference

[sjakobi]

Context: #364

---

TODO:

• How does the delete refactoring affect its performance?
• delete'' -> deleteSubTree
• What happens when the internal delete''.go function is removed and we recurse to delete'' itself?

[sjakobi]

Wow, the -fdebug assertions are paying off, I guess!

         valid:                             FAIL
          *** Failed! (after 54 tests and 15 shrinks):
          Exception:
            Data.HashMap.Internal.Array.shrink: Check failed:  _n >  (0 :: Int) (0 vs. 0)
            CallStack (from HasCallStack):
              error, called at ./Data/HashMap/Internal/Array.hs:225:39 in unordered-containers-0.2.20.1-inplace:Data.HashMap.Internal.Array
          fromList [(K {hash = -1, _x = B},0),(K {hash = -1, _x = A},0)]
          fromList [(K {hash = -1, _x = A},0),(K {hash = -1, _x = B},0)]
          Exception thrown while showing test case:
            Data.HashMap.Internal.Array.shrink: Check failed:  _n >  (0 :: Int) (0 vs. 0)
            CallStack (from HasCallStack):
              error, called at ./Data/HashMap/Internal/Array.hs:225:39 in unordered-containers-0.2.20.1-inplace:Data.HashMap.Internal.Array
          
          Use --quickcheck-replay="(SMGen 2214100628715094315 10962607274683585473,53)" to reproduce.
          Use -p '/Data.HashMap.Strict.difference.valid/' to rerun this test only.

[sjakobi]

So the failing assertion is this one, introduced in b73381e:

unordered-containers/Data/HashMap/Internal/Array.hs

Lines 221 to 228 in 59ddae5

	-- | The returned array is the same as the array given, as it is shrunk in place.
	shrink :: MArray s a -> Int -> ST s (MArray s a)
	shrink mary _n@(I# n#) =
	CHECK_GT("shrink", _n, (0 :: Int))
	CHECK_LE("shrink", _n, (unsafeLengthM mary))
	ST $ \s -> case Exts.shrinkSmallMutableArray# (unMArray mary) n# s of
	s' -> (# s', mary #)
	{-# INLINE shrink #-}

I don't see an obvious technical reason why an array shouldn't be shrunk to length 0, apart from the fact that an array of length 0 is useless for storing elements. In the case of the new filter function, an array of length 0 seems like a perfectly valid result.

So I'll just change CHECK_GT to CHECK_GE.

[treeowl]

I agree that assertion is bogus.

[sjakobi]

difference is a lot faster than before:

master:

All
  HashMap.Strict
    difference
      disjoint
        Int
          0:      OK
            6.53 ns ± 164 ps
          1:      OK
            13.2 ns ± 400 ps
          10:     OK
            123  ns ± 1.3 ns
          100:    OK
            1.98 μs ±  23 ns
          1000:   OK
            24.4 μs ± 246 ns
          10000:  OK
            307  μs ± 5.7 μs
          100000: OK
            3.86 ms ±  92 μs
      overlap
        Int
          0:      OK
            6.08 ns ±  60 ps
          1:      OK
            6.11 ns ± 104 ps
          10:     OK
            133  ns ± 2.3 ns
          100:    OK
            1.83 μs ±  55 ns
          1000:   OK
            23.6 μs ± 928 ns
          10000:  OK
            302  μs ±  11 μs
          100000: OK
            3.85 ms ±  25 μs
      equal
        Int
          0:      OK
            5.55 ns ± 162 ps
          1:      OK
            9.63 ns ± 364 ps
          10:     OK
            181  ns ± 6.2 ns
          100:    OK
            1.86 μs ±  51 ns
          1000:   OK
            21.9 μs ± 658 ns
          10000:  OK
            212  μs ± 6.2 μs
          100000: OK
            2.75 ms ±  88 μs

This branch:

All
  HashMap.Strict
    difference
      disjoint
        Int
          0:      OK
            4.92 ns ± 122 ps
          1:      OK
            7.30 ns ± 102 ps
          10:     OK
            37.0 ns ± 1.4 ns
          100:    OK
            295  ns ±  11 ns
          1000:   OK
            3.02 μs ±  80 ns
          10000:  OK
            40.4 μs ± 979 ns
          100000: OK
            541  μs ± 8.0 μs
      overlap
        Int
          0:      OK
            4.88 ns ± 148 ps
          1:      OK
            4.86 ns ± 192 ps
          10:     OK
            96.6 ns ± 2.7 ns
          100:    OK
            912  ns ±  26 ns
          1000:   OK
            9.69 μs ± 290 ns
          10000:  OK
            135  μs ± 3.2 μs
          100000: OK
            1.91 ms ±  60 μs
      equal
        Int
          0:      OK
            4.34 ns ± 114 ps
          1:      OK
            6.81 ns ± 258 ps
          10:     OK
            7.95 ns ± 284 ps
          100:    OK
            7.65 ns ± 236 ps
          1000:   OK
            7.58 ns ± 164 ps
          10000:  OK
            7.66 ns ± 222 ps
          100000: OK
            7.60 ns ± 252 ps

[sjakobi]

I think the complexity is still the same. For example, if we do something like difference a large_superset_of_a, we end up checking each leaf of a against the superset.

[treeowl]

Isn't it always 0 or 1, making the third branch unreachable? I suspect this part can be cleaned up by using let l = ... instead of binding that in a case branch.

[sjakobi]

The third branch will be used when l isn't a Leaf or Collision.

I have opened #528 to clean up this pattern, and I'll eventually return to that PR! :)

[treeowl]

Can't we just do Leaf h (A.index v (1 - i))?

[sjakobi]

I think we can. But that's what's #528 is about.