From 26470a91c7e495aeefea109f7767b45486d4ae5c Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 1 Jun 2021 22:50:59 -0700 Subject: [PATCH] fix #17898 initRand now uses monotonic time to guarantee uniqueness --- lib/pure/random.nim | 11 ++----- lib/std/monotimes.nim | 62 +++++++++++++++++-------------------- tests/stdlib/tmonotimes.nim | 5 +-- tests/stdlib/trandom.nim | 27 +++++++++++++++- 4 files changed, 60 insertions(+), 45 deletions(-) diff --git a/lib/pure/random.nim b/lib/pure/random.nim index 6a5f35bdeafb7..1077e65fff640 100644 --- a/lib/pure/random.nim +++ b/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 diff --git a/lib/std/monotimes.nim b/lib/std/monotimes.nim index bf7ee05c89212..be5cdfd9ddfa2 100644 --- a/lib/std/monotimes.nim +++ b/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: "".} -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`. diff --git a/tests/stdlib/tmonotimes.nim b/tests/stdlib/tmonotimes.nim index aa20feba8df03..2a5fe342e4e84 100644 --- a/tests/stdlib/tmonotimes.nim +++ 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.. a diff --git a/tests/stdlib/trandom.nim b/tests/stdlib/trandom.nim index e47ddad666039..5ec3f9ba676ac 100644 --- a/tests/stdlib/trandom.nim +++ b/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..