better implementation, distinguish int32 vs int64; add performance regression test

Author: timotheecour

compiler/vmops.nim

@@ -18,7 +18,7 @@ from md5 import getMD5
 from sighashes import symBodyDigest
 from times import cpuTime
 
-from hashes import hash, hashBiggestInt
+from hashes import hash, hashUInt64, hashUInt32
 
 template mathop(op) {.dirty.} =
   registerCallback(c, "stdlib.math." & astToStr(op), `op Wrapper`)
@@ -185,8 +185,10 @@ proc registerAdditionalOps*(c: PCtx) =
 
   registerCallback c, "stdlib.hashes.hashVmImpl", hashVmImpl
 
-  registerCallback c, "stdlib.hashes.hashBiggestInt", proc (a: VmArgs) {.nimcall.} =
-    a.setResult hashBiggestInt(getInt(a, 0))
+  registerCallback c, "stdlib.hashes.hashUInt64", proc (a: VmArgs) {.nimcall.} =
+    a.setResult hashUInt64(cast[uint64](getInt(a, 0)))
+  registerCallback c, "stdlib.hashes.hashUInt32", proc (a: VmArgs) {.nimcall.} =
+    a.setResult hashUInt32(cast[uint32](getInt(a, 0)))
 
   proc hashVmImplByte(a: VmArgs) =
     # nkBracket[...]

lib/pure/hashes.nim

@@ -89,18 +89,44 @@ proc hashData*(data: pointer, size: int): Hash =
     dec(s)
   result = !$h
 
-proc hash*(x: string): Hash {.noSideEffect.}
+when defined(js):
+  proc hash*(x: string): Hash {.noSideEffect.}
 
-proc hashBiggestInt*(x: BiggestInt): Hash {.inline.} =
+proc hashUInt64*(x: uint64): Hash {.inline.} =
   ## for internal use; user code should prefer `hash` overloads
   when nimvm: # in vmops
     doAssert false
   else:
-    when defined(js):
-      # could use BigInt, see https://github.com/nim-lang/RFCs/issues/187
-      hash($x)
-    else:
-      hashData(cast[pointer](unsafeAddr x), type(x).sizeof)
+    # would orders of magnitude worse, see thashes_perf toHighOrderBits
+    # faster than using murmurhash or Jenkins via
+    # hashData(cast[pointer](unsafeAddr x), type(x).sizeof)
+
+    # would a bit worse, see thashes_perf toInt64
+    # type ByteArr = array[int64.sizeof, uint8]
+    # result = murmurHash(cast[ptr ByteArr](unsafeAddr x)[])
+
+    # inspired from https://gist.github.com/badboy/6267743#64-bit-mix-functions
+    var x = x
+    x = (not x) + (x shl 21) # x = (x shl 21) - x - 1;
+    x = x xor (x shr 24)
+    x = (x + (x shl 3)) + (x shl 8) # x * 265
+    x = x xor (x shr 14)
+    x = (x + (x shl 2)) + (x shl 4) # x * 21
+    x = x xor (x shr 28)
+    x = x + (x shl 31)
+    result = cast[Hash](x)
+
+proc hashUInt32*(x: uint32): Hash {.inline.} =
+  ## for internal use; user code should prefer `hash` overloads
+  # calling `hashUInt64(x)` would perform 1.736 worse, see thashes_perf toInt32
+  when nimvm: # in vmops
+    doAssert false
+  else:
+    # inspired from https://gist.github.com/badboy/6267743
+    var x = x xor ((x shr 20) xor (x shr 12))
+    # debugEcho ("hashUInt32", x)
+    # return cast[Hash](x xor (x shr 7) xor (x shr 4))
+    result = cast[Hash](x xor (x shr 7) xor (x shr 4))
 
 proc hash*[T: SomeNumber | Ordinal | char](x: T): Hash {.inline.} =
   ## Efficient hashing of numbers, ordinals (eg enum), char.
@@ -117,14 +143,16 @@ proc hash*[T: SomeNumber | Ordinal | char](x: T): Hash {.inline.} =
       # bugfix: the previous code was using `x = x + 1.0` (presumably for
       # handling negative 0), however this leads to collisions for small x due
       # to FP finite precision.
-      let x = block:
-        when sizeof(BiggestInt) == sizeof(T):
-          cast[BiggestInt](x + T(0))
-        else: # for nimvm
-          cast[int32](x + T(0)).BiggestInt
+      let x = x + T(0)
+    when defined(js):
+      when nimvm:
+        # this could also be `hashUInt64(cast[uint64](x))` on a 32 bit machine,
+        # but we can't query for int.sizeof since that's hardcoded for nim js
+        hashUInt64(cast[uint64](x))
+      else: hash($x.float)
     else:
-      let x = x.BiggestInt
-    hashBiggestInt(x)
+      when sizeof(T) == sizeof(uint64): hashUInt64(cast[uint64](x))
+      else: hashUInt32(cast[uint32](x))
   else:
     # empirically better for small types, the collision risk is limited anyway
     # due to cardinality of at most 2^16=65536

tests/collections/tcollections_to_string.nim

@@ -7,6 +7,7 @@ import tables
 import deques
 import lists
 import critbits
+import sequtils, algorithm
 
 # Tests for tuples
 doAssert $(1, 2, 3) == "(1, 2, 3)"
@@ -26,9 +27,11 @@ doAssert $(toOrderedSet([1, 2, 3])) == "{1, 2, 3}"
 doAssert $(toOrderedSet(["1", "2", "3"])) == """{"1", "2", "3"}"""
 doAssert $(toOrderedSet(['1', '2', '3'])) == """{'1', '2', '3'}"""
 
+proc toStrSorted(t: Table): string = $(toSeq(pairs(t)).sorted)
+
 # Tests for tables
-doAssert $({1: "1", 2: "2"}.toTable) == """{1: "1", 2: "2"}"""
-doAssert $({"1": 1, "2": 2}.toTable) == """{"1": 1, "2": 2}"""
+doAssert {1: "1", 2: "2"}.toTable.toStrSorted == """@[(1, "1"), (2, "2")]"""
+doAssert {"1": 1, "2": 2}.toTable.toStrSorted == """@[("1", 1), ("2", 2)]"""
 
 # Tests for deques
 block:

tests/collections/thashes_perf.nim

@@ -0,0 +1,77 @@
+discard """
+  cmd: "nim $target -d:danger -r $file"
+  targets: "c cpp js"
+  joinable: false
+  timeout: 100.0
+"""
+
+## performance regression tests, refs: #13393 and #11764
+## this takes into account insertion/retrieval time, plus time to compute hashes
+## in various input distributions to ensure we have both high quality hashes
+## (few collisions) as well as speedy hash computations.
+## We run this benchmark on: c, cpp, js, vm targets to ensure all cases are
+## covered.
+##
+## The spec timeout is a very generous upperbound; it's needed just in case
+## a regression causes CI to take forever on this test.
+
+import times, tables
+
+proc toInt64(a: int): int64 = cast[int64](a)
+proc toInt32(a: int): int32 = cast[int32](a)
+proc toHighOrderBits(a: int): uint64 =
+  result = cast[uint64](a) shl 32
+
+proc toSmallFloat(a: int): float =
+  result = a.float*1e-20
+
+# block ttables_perf:
+template fun() =
+  proc TestHashIntInt[Fun](fun: Fun) =
+    type T = type(fun(1))
+    var tab = initTable[T, string]()
+    const n = 100_000 # use that
+    for i in 1..n:
+      let h = fun(i)
+      doAssert h notin tab
+      tab[h] = $i
+      doAssert tab[h] == $i
+
+  proc runTimingImpl[Fun](fun: Fun, name: string, timeout: float) =
+    ## return elapsed time in seconds
+    type T = type(fun(1))
+    echo "perf test begin: T=" & $T & " " & name
+
+    template epochTime2(): untyped =
+      when nimvm: 0.0 # epochTime not defined in vm
+      else: times.epochTime()
+
+    let eTime1 = epochTime2()
+
+    var numIter = 50
+    when defined(js): numIter = 10
+    when nimvm: numIter = 1
+    else: discard
+
+    for i in 1 .. numIter:
+      TestHashIntInt(fun)
+    let eTime2 = epochTime2()
+    let runtime = eTime2 - eTime1
+    echo "pref test done: runtime: ", runtime, " timeout: ", timeout
+    doAssert runtime < timeout # performance regression check, don't make it too tight for CI
+
+  template runTiming(fun, timout) = runTimingImpl(fun, astToStr(fun), timout)
+  # leaving some large buffer for timing, but not too much to catch performance regressions
+  let timeouts = when defined(js): [8.0, 8.0, 8.0] else: [4.0, 3.0, 5.0]
+  # on my local OSX machine I get: [1.44, 0.85, 1.2316, 1.38] for `nim c` and ~1.8 for `nim js`
+  # azure CI OSX I get: [2.8, 1.5, 2.98] for `nim c`
+
+  runTiming toInt64, timeouts[0]
+  runTiming toInt32, timeouts[1]
+  runTiming toHighOrderBits, timeouts[2]
+  when false: # disabled to save CI time but easy to turn back on for deubugging
+    runTiming toSmallFloat, 3.0
+
+when defined(c) or defined(js): # skip cpp (a bit redundant with c in this case)
+  static: fun()
+fun()

tests/collections/ttables.nim

@@ -164,7 +164,7 @@ block tableconstr:
 
 block ttables2:
   proc TestHashIntInt() =
-    var tab = initTable[int,int]()
+    var tab = initTable[int, int]()
     const n = 1_000_000 # bottleneck: 50 seconds on OSX in debug mode
     for i in 1..n:
       tab[i] = i
@@ -185,7 +185,6 @@ block ttables2:
     delTab.del(i)
   delTab[5] = 5
 
-
   run1()
   echo "2"