fix #17898 initRand now uses monotonic time to guarantee uniqueness

lib/pure/random.nim

@@ -604,25 +604,20 @@ proc shuffle*[T](x: var openArray[T]) =
   shuffle(state, x)
 
 when not defined(nimscript) and not defined(standalone):
-  import times
+  import std/monotimes
 
   proc initRand(): Rand =
     ## Initializes a new Rand state with a seed based on the current time.
     ##
     ## The resulting state is independent of the default RNG's state.
     ##
-    ## **Note:** Does not work for NimScript or the compile-time VM.
+    ## **Note:** In VM, requires `--experimental:vmops`
     ##
     ## See also:
     ## * `initRand proc<#initRand,int64>`_ that accepts a seed for a new Rand state
     ## * `randomize proc<#randomize>`_ that initializes the default RNG using the current time
     ## * `randomize proc<#randomize,int64>`_ that accepts a seed for the default RNG
-    when defined(js):
-      let time = int64(times.epochTime() * 1000) and 0x7fff_ffff
-      result = initRand(time)
-    else:
-      let now = times.getTime()
-      result = initRand(convert(Seconds, Nanoseconds, now.toUnix) + now.nanosecond)
+    result = initRand(getMonoTime().ticks)
 
   since (1, 5, 1):
     export initRand

lib/std/monotimes.nim

@@ -38,16 +38,9 @@ See also
 
 import times
 
-when defined(js):
-  import std/jsbigints
-  type Tick = JsBigInt
-else:
-  type Tick = int64
-
 type
   MonoTime* = object ## Represents a monotonic timestamp.
-    ticks: Tick
-
+    ticks: int64
 
 when defined(macosx):
   type
@@ -59,27 +52,28 @@ when defined(macosx):
   proc mach_timebase_info(info: var MachTimebaseInfoData) {.importc,
     header: "<mach/mach_time.h>".}
 
-when defined(js) and defined(nodejs):
-  proc getJsTicks: Tick =
+when defined(js):
+  proc getJsTicks: float =
     ## Returns ticks in nanoseconds.
-    {.emit: """
-    let process = require('process');
-    `result` = process.hrtime.bigint();
-    """.}
-
-elif defined(js):
-  proc jsNow(): Tick {.importjs: "window.performance.now()".}
-  proc getJsTicks: Tick =
-    ## Returns ticks in the unit nanoseconds, with up to 5us precision
-    result = JsBigInt(jsNow()) * 1_000_000'big
-
-  # # Workaround for #6752.
-  # {.push overflowChecks: off.}
-  # proc `-`(a, b: int64): int64 =
-  #   system.`-`(a, b)
-  # proc `+`(a, b: int64): int64 =
-  #   system.`+`(a, b)
-  # {.pop.}
+    # xxx instead, use JsBigInt throughout the API
+    # to avoid `overflowChecks: off` and provide higher precision, but this
+    # requires some care, e.g. because of `proc low*(typ: typedesc[MonoTime]): MonoTime =`
+    when defined(nodejs):
+      {.emit: """
+      let process = require('process');
+      `result` = Number(process.hrtime.bigint());
+      """.}
+    else:
+      proc jsNow(): float {.importjs: "window.performance.now()".}
+      result = jsNow() * 1e6
+
+  # Workaround for #6752.
+  {.push overflowChecks: off.}
+  proc `-`(a, b: int64): int64 =
+    system.`-`(a, b)
+  proc `+`(a, b: int64): int64 =
+    system.`+`(a, b)
+  {.pop.}
 
 elif defined(posix) and not defined(osx):
   import posix
@@ -99,7 +93,7 @@ proc getMonoTime*(): MonoTime {.tags: [TimeEffect].} =
   ## See [MDN](https://developer.mozilla.org/en-US/docs/Web/API/Performance/now)
   ## for more information.
   when defined(js):
-    result = MonoTime(ticks: getJsTicks())
+    result = MonoTime(ticks: getJsTicks().int64)
   elif defined(macosx):
     let ticks = mach_absolute_time()
     var machAbsoluteTimeFreq: MachTimebaseInfoData
@@ -120,25 +114,25 @@ proc getMonoTime*(): MonoTime {.tags: [TimeEffect].} =
     let queryPerformanceCounterFreq = 1_000_000_000'u64 div freq
     result = MonoTime(ticks: (ticks * queryPerformanceCounterFreq).int64)
 
-proc ticks*(t: MonoTime): int64 =
+proc ticks*(t: MonoTime): int64 {.inline.} =
   ## Returns the raw ticks value from a `MonoTime`. This value always uses
   ## nanosecond time resolution.
-  int64(t.ticks)
+  t.ticks
 
 proc `$`*(t: MonoTime): string =
   $t.ticks
 
 proc `-`*(a, b: MonoTime): Duration =
   ## Returns the difference between two `MonoTime` timestamps as a `Duration`.
-  initDuration(nanoseconds = int64(a.ticks - b.ticks))
+  initDuration(nanoseconds = (a.ticks - b.ticks))
 
 proc `+`*(a: MonoTime, b: Duration): MonoTime =
   ## Increases `a` by `b`.
-  MonoTime(ticks: a.ticks + b.inNanoseconds.big)
+  MonoTime(ticks: a.ticks + b.inNanoseconds)
 
 proc `-`*(a: MonoTime, b: Duration): MonoTime =
   ## Reduces `a` by `b`.
-  MonoTime(ticks: a.ticks - b.inNanoseconds.big)
+  MonoTime(ticks: a.ticks - b.inNanoseconds)
 
 proc `<`*(a, b: MonoTime): bool =
   ## Returns true if `a` happened before `b`.

tests/stdlib/tmonotimes.nim

@@ -22,8 +22,9 @@ template main =
     doAssert low(MonoTime) < t1
 
   block:
-    for i in 0..<1000000: # test should take ~ 1sec
-      # this would fail with getTime instead of getMonoTime, as expected
+    const n = when defined(js): 20000 else: 1000000 # keep test under ~ 1sec
+    for i in 0..<n:
+      # this could fail with getTime instead of getMonoTime, as expected
       let a = getMonoTime()
       let b = getMonoTime()
       doAssert b > a

tests/stdlib/trandom.nim

@@ -1,9 +1,12 @@
 discard """
   joinable: false # to avoid messing with global rand state
   targets: "c js"
+  matrix: "; -d:danger" # this matters because of the `#17898` test
 """
 
-import std/[random, math, os, stats, sets, tables]
+import std/[random, math, stats, sets, tables]
+when not defined(js):
+  import std/os
 
 randomize(233)
 
@@ -187,3 +190,25 @@ block: # bug #17467
     doAssert x > 1e-4, $(x, i)
       # This used to fail for each i in 0..<26844, i.e. the 1st produced value
       # was predictable and < 1e-4, skewing distributions.
+
+block: # bug #17898
+  let size = 1000
+  var vals = newSeq[Rand](size) 
+  for i in 0..<size:
+    vals[i] = initRand()
+    # this should do as little as possible besides calling initRand to
+    # ensure the test is meaningful
+  # proc isUnique[T](a: iterable[T]): bool =
+  template isUnique[T](a: iterable[T]): bool =
+    # xxx move to std/algorithm
+    var s: HashSet[T]
+    var ret = true
+    for ai in a:
+      if ai in s:
+        ret = false
+        break
+      else:
+        s.incl ai
+    ret
+
+  doAssert isUnique(items(vals))