Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc times: do not use now (and also epochTime) for benchmarking #17405

Merged
merged 3 commits into from
Mar 18, 2021

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Mar 17, 2021

common pitfall, eg now is used in nim-lang/website#274

rationale:

  • epochTime uses (on osx) gettimeofday which is non-monotonic, eg see [1]
  • now has same problems as epochTime, and on top of that has overhead (which is really bad for benchmarking, depending on the time scales involved), and deals with timezones, and return type DateTime is a heavy object;

links

gettimeofday() is totally inappropriate for benchmarking. Any time the system clock is being adjusted by NTP, for instance, your benchmark timing will be skewed.They should be using the following API if they're going to use the system time to measure time differences:clock_gettime(CLOCK_MONOTONIC, &timespec);It's a really good clue that a timing function is inappropriate for benchmarking when the man page talks about time zones.

future work

CLOCK_MONOTONIC_COARSE (since Linux 2.6.32; Linux-specific)
A faster but less precise version of CLOCK_MONOTONIC. Use when you need very fast, but not fine-grained timestamps.
CLOCK_MONOTONIC_RAW (since Linux 2.6.28; Linux-specific)
Similar to CLOCK_MONOTONIC, but provides access to a raw hardware-based time that is not subject to NTP adjustments or the incremental adjustments performed by adjtime(3).
CLOCK_BOOTTIME (since Linux 2.6.39; Linux-specific)
Identical to CLOCK_MONOTONIC, except it also includes any time that the system is suspended. This allows applications to get a suspend-aware monotonic clock without having to deal with the complications of CLOCK_REALTIME, which may have discontinuities if the time is changed using settimeofday(2).
CLOCK_PROCESS_CPUTIME_ID
High-resolution per-process timer from the CPU.
CLOCK_THREAD_CPUTIME_ID
Thread-specific CPU-time clock.

@timotheecour timotheecour changed the title doc times: do not use now (and also epochTime) for benchmarking doc times: do not use now (and also epochTime) for benchmarking Mar 17, 2021
lib/pure/times.nim Outdated Show resolved Hide resolved
lib/pure/times.nim Outdated Show resolved Hide resolved
timotheecour and others added 2 commits March 17, 2021 16:12
Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
@Araq Araq merged commit 4dd34fe into nim-lang:devel Mar 18, 2021
@timotheecour timotheecour deleted the pr_doc_now_bench branch March 18, 2021 17:51
ringabout pushed a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
…im-lang#17405)

* doc times: do not use now for benchmarking

* Update lib/pure/times.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

* Update lib/pure/times.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
…im-lang#17405)

* doc times: do not use now for benchmarking

* Update lib/pure/times.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

* Update lib/pure/times.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants