bpo-31415: Add _PyTime_GetPerfCounter() and use it for -X importtime

[vstinner]

• Add _PyTime_GetPerfCounter()
• Use _PyTime_GetPerfCounter() for -X importtime

https://bugs.python.org/issue31415

[vstinner]

The drawback of this change is that time.perf_counter() now converts QueryPerformanceCounter() / QueryPerformanceFrequency() double into a _PyTime_t (integer) and then back to double. Two useless conversions required by the standard _PyTime_t format.

_PyTime_t has a resolution of 1 nanoscond, so the conversions should not loose precision. _PyTime_GetWinPerfCounter() reads the first value of QueryPerformanceCounter() to start at t=0.0, so numbers should be "small" (lower than 1 day in the common case, lower than 1 year in most cases).

[terryjreedy]

I recompiled after pulling this PR. This solves the problem I reported on the issue. I get a report for traceback similar to that in the opening post (with times multiplied by +- 4.5 for debug and slower machine). Idlelib report also looks sensible and useful. I re-added skip_news to trigger Bedevere. You might want to close and open to try get Appveyor to run. However, all tests ran ok on my Windows machine.

I read C code but cannot properly review it.

[methane]

LGTM

[ncoghlan]

The general approach seems sound, but I have some ideas that may make it possible to avoid the negative side effect on time.perf_counter()

[ncoghlan]

And this accepted a double *t as the output parameter?

[methane]

+1

[ncoghlan]

And the call to _PyTime_FromDouble was moved into this wrapper function?

It wouldn't be ideal, since the #ifdef logic would exist in two places, but maybe that duplication could be handled via a common macro definition that was used in both places, rather than by incurring the redundant conversion at runtime on Windows.

[ncoghlan]

Idea for avoiding the redundant double -> _PyTime_t -> double conversion in time.perf_counter():

What if this still kept a compile time conditional branch between the Windows API and the *nix one?

[ncoghlan]

Heh, I think the main review page sorts by file path, while the comment preview sorts by file name, so my comments appear out of order in the summary :)

[vstinner]

In short, I tested and the double => _PyTime_t => double conversions don't loose any nanosecond.

The drawback of this change is that time.perf_counter() now converts QueryPerformanceCounter() / QueryPerformanceFrequency() double into a _PyTime_t (integer) and then back to double. Two useless conversions required by the standard _PyTime_t format.

I wrote "rounding tests" manually. I tested that _PyTime_AsSecondsDouble(_PyTime_FromDouble(t)))==t for various values. I tested with the time.perf_counter() raw values + 5 years in seconds: we don't see a single nanosecond.

_PyTime_t type is an integer used to store timestamps with a resolution of 1 nanosecond. The type is big enough to store a timestamp in the range [-292 years; 292 years].

The _PyTime_AsSecondsDouble() function tries to reduce the precision loss. This function doesn't take a rounding mode parameter since I'm not aware of any rounding issue.

_PyTime_FromDouble() is quite simple and is used with _PyTime_ROUND_FLOOR rounding mode: "Round towards minus infinity (-inf). For example, used to read a clock."

Last year, I wrote an article on the _PyTime API: https://haypo.github.io/pytime.html

[ncoghlan]

Note: my suggestions about aiming to avoid the double->int->double step weren't related to potentially losing precision, they were related to that conversion dance making time.perf_counter() slower than it needs to be.

(I don't think it's a big deal either way, it just seemed worth mentioning since these are all internal APIs anyway)

[vstinner]

Idea for avoiding the redundant double -> _PyTime_t -> double conversion in time.perf_counter(): (...)

I wrote PR #3942 to finish to convert _PyTime_GetWinPerfCounter() to _PyTime_t: it doesn't use C double anymore. It does all maths with pure integers. It should avoid any loss of precision.