-
Notifications
You must be signed in to change notification settings - Fork 236
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
std::chrono wrappers for Windows times and timestamps #89
base: master
Are you sure you want to change the base?
Conversation
How do I sign the CLA? I keep pressing "Sign in with GitHub to agree" and looping back to the same page |
[As requested in the comments to PR microsoft#89](microsoft#89 (comment))
[As requested in the comments to PR microsoft#89](microsoft#89 (comment))
Changed template parameter names from Pascal case to snake case, with _t suffixes where appropriate Split function template parameters into a type parameter and a value parameter; as a result, the details::QueryUnbiasedInterruptTime wrapper is now unnecessary [As requested in the comments to PR microsoft#89](microsoft#89 (comment))
Done, done and done. I kept the definition of the |
I still have issues with this, please advise |
Perhaps your browser privacy settings, maybe try in another browser's clean profile |
I'm not having issues either. Perhaps try with a different browser, disable ad blocker, try an incognito window, etc. to see if that helps out |
Tried all of those, same issue. Who do I report issues with the CLA Assistant to? |
From my understanding that page is to authorize a GitHub application. The page for managing GitHub applications for your profile has a link to https://help.github.com/en/articles/authorizing-oauth-apps, which might be helpful? |
include/wil/chrono.h
Outdated
using period = std::milli; | ||
using duration = std::chrono::duration<rep, period>; | ||
using time_point = std::chrono::time_point<base_clock_t, duration>; | ||
static constexpr bool const is_steady = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for integral type constexpr
with const
together is useless, so we can remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
constexpr always implies const for variables.
…On Fri, 25 Oct 2019 at 11:48, soroshsabz ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In include/wil/chrono.h
<#89 (comment)>:
> +#include <sysinfoapi.h> // GetTickCount[64], GetSystemTime[Precise]AsFileTime
+#include <realtimeapiset.h> // Query[Unbiased]InterruptTime[Precise]
+
+namespace wil
+{
+#pragma region std::chrono wrappers for GetTickCount[64]
+ namespace details
+ {
+ template<typename rep_t, typename get_tick_count_fn_t, get_tick_count_fn_t get_tick_count_fn, typename base_clock_t>
+ struct tick_count_clock_impl
+ {
+ using rep = rep_t;
+ using period = std::milli;
+ using duration = std::chrono::duration<rep, period>;
+ using time_point = std::chrono::time_point<base_clock_t, duration>;
+ static constexpr bool const is_steady = true;
I think for integral type constexpr with const together is useless, so we
can remove it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#89?email_source=notifications&email_token=ABRELNQX5XRCVOFMS67C2C3QQMIMZA5CNFSM4ISXW7NKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJIPOIA#pullrequestreview-307296032>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRELNQOWBT6X5WFNQQZFBDQQMIMZANCNFSM4ISXW7NA>
.
|
Added the inline specifier to the inline functions that were missing it
Don't mix typename and class in template parameter lists
Delimit the code regions for thread/process times functions and thread/process CPU time clocks
In the non-template overload of current_thread_cpu_time_clock::now, call the non-template overload of get_thread_cpu_time; in the non-template overload of current_process_cpu_time_clock::now, call the non-template overload get_process_cpu_time. This makes sure the two methods inherit the error policy of the underlying functions
constexpr implies const
Alright, I removed the redundant Also I finally signed the CLA! |
using period = std::ratio_multiply<std::hecto, std::nano>; | ||
using duration = std::chrono::duration<rep, period>; | ||
using time_point = std::chrono::time_point<base_clock_t, duration>; | ||
static constexpr bool is_steady = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrasing from the documentation:
The unbiased interrupt-time count does not include time the system spends in sleep or hibernation.
Makes it sound like this should be false
, at least for QueryUnbiasedInterruptTime
. At least the standard makes it sound like it should be:
true
ift1 <= t2
is alwaystrue
and the time between clock ticks is constant, otherwisefalse
.
return time_point{duration{ static_cast<rep>(filetime_to_int(ft)) }}; | ||
} | ||
|
||
#if __WI_LIBCPP_STD_VER > 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can assume at least C++14
#pragma endregion | ||
|
||
#pragma region Thread/process CPU clocks | ||
struct current_thread_cpu_time_clock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requires is_steady
static member to satisfy Clock requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with current_process_cpu_time_clock
and possibly other I may have missed
Hey @hackbunny - are you still interested in pushing forward on this? I'm happy to apply @dunhor's feedback and push to completion. |
@jonwis please and thank you. Go ahead, I had completely forgotten about this. |
Is there anything else this PR needs before it can be merged? |
I hope this PR is merge quickly :) |
New header
<wil/chrono.h>
definesstd::chrono
adapters for all fixed-frequency Windows timers and timestampsGeneral considerations
User mode-only for now, because I don't know if
<chrono>
is kernel mode-safe. If we could get confirmation of the safety of<chrono>
from the compiler people, we could add similar wrappers for functions likeKeQueryTickCount
,KeQuerySystemTime
, etc. Alternatively, we could fork the LLVM<chrono>
into awistd_chrono.h
headerWhere possible, functions have been made
constexpr
through__WI_LIBCPP_CONSTEXPR_AFTER_CXX11
, so that it depends on the underlyingconstexpr
-ness of<chrono>
functionsI haven't implemented DOS/FAT timestamps because, frankly, I think they aren't worth the effort
The following timers have a variable frequency and can't have zero-overhead
std::chrono
adapters, so they aren't included for now:QueryPerformanceCounter
The auxiliary counter... if there was a function to query it (what is the deal with it?)
The CPU timestamp register, as returned by intrinsics like
__rdtsc
or__rdtscp
. This timer comes with additional considerations, such as:Detecting availability: whether the architecture implements it, whether the sandbox/hypervisor/VM allows access to it, whether it has a stable frequency (e.g. constant or invariant TSC on Intel/AMD processors) etc.
Querying the frequency: hard (I tried! and I still don't have a good answer)
Instruction ordering: for example,
__rdtsc
is unordered, and needs a fence (_mm_mfence
or_mm_lfence
) for meaningful measurements, or alternatively__rdtscp
can be used; but all solutions require an additional feature test... and even the feature test itself (__cpuid
/__cpuidex
) may, in turn, require a feature testProcessor cycle counters (e.g.
QueryThreadCycleTime
) are out of scope, since they aren't time-based at allTesting
I don't know how to write test cases, but I could learn. All clock classes support a form of compile-time dependency injection, so writing tests for corner cases shouldn't be hard
Overview
Tick counter clocks
The following Clocks are defined:
tick_count_clock
:rep
:DWORD
period
:std::milli
(1ms)is_steady
:true
now
: wrapsGetTickCount
(
#if _WIN32_WINNT >= 0x0600
)tick_count64_clock
:rep
:ULONGLONG
period
,is_steady
,time_point
: same astick_count_clock
now
: wrapsGetTickCount64
Caveats
Unlike standard C++ clocks, these clocks have an unsigned
rep
. This is intentional: the value returned byGetTickCount
has a limited range, and we don't want to incur in undefined behavior dealing with signed under/overflowSystem time clocks
The following Clocks are defined:
system_time_clock
:rep
:LONGLONG
period
:std::ratio_multiply<std::hecto, std::nano>
(100ns)is_steady
:false
now
: wrapsGetSystemTimeAsFileTime
to_filetime
andfrom_filetime
: conversion betweentime_point
and rawFILETIME
sto_system_clock
andfrom_system_clock
: conversion betweentime_point
andstd::chrono::system_clock::time_point
to_time_t
,from_time_t
,to_time32_t
,from_time32_t
,to_time64_t
,from_time64_t
: conversion betweentime_point
(100ns, Windows epoch) andstd::time_t
/__time32_t
/__time64_t
(1s, UNIX epoch)(
#if _WIN32_WINNT >= _WIN32_WINNT_WIN8
)precise_system_time_clock
:rep
,period
,time_point
,is_steady
: same assystem_time_clock
to_filetime
,from_filetime
,to_system_clock
,from_system_clock
,to_time_t
,from_time_t
,to_time32_t
,from_time32_t
,to_time64_t
,from_time64_t
: same assystem_time_clock
now
: wrapsGetSystemTimePreciseAsFileTime
high_precision_system_time_clock
:(
#if _WIN32_WINNT >= _WIN32_WINNT_WIN8
) alias forprecise_system_time_clock
(
#else
) alias forsystem_time_clock
Caveats
to_filetime
andfrom_filetime
are implemented in-line, because includingwin32_helpers.h
to usewil::filetime
routines didn't seem worth it compared to the downsides:win32_helpers.h
pulls in extra dependencies and pollutes the namespacewil::filetime
routines aren'tconstexpr
(norconstexpr
-compatible)In
to_system_clock
andfrom_system_clock
, we non-portably assumestd::chrono::system_clock::time_point
starts from the UNIX epoch. However, this is true in all C++ runtime library implementations I know of and it will be a requirement starting from C++20Interrupt time clocks
When targeting Windows 7 or later (
#if _WIN32_WINNT >= 0x0601
), the following Clock is defined:unbiased_interrupt_time_clock
:rep
:LONGLONG
period
:std::ratio_multiply<std::hecto, std::nano>
(100ns)is_steady
:true
now
: wrapsQueryUnbiasedInterruptTime
When targeting Windows 7 or later and using a Windows 10 or later SDK (
#ifdef NTDDI_WIN10
), the following Clocks are also defined:interrupt_time_clock
:rep
:LONGLONG
period
:std::ratio_multiply<std::hecto, std::nano>
(100ns)is_steady
:true
now
: wrapsQueryInterruptTime
precise_interrupt_time_clock
:rep
,period
,time_point
,is_steady
: same asinterrupt_time_clock
now
: wrapsQueryInterruptTimePrecise
precise_unbiased_interrupt_time_clock
:rep
,period
,time_point
,is_steady
: same asunbiased_interrupt_time_clock
now
: wrapsQueryUnbiasedInterruptTimePrecise
Process and thread times
Types
The following types are defined:
cpu_time_duration
:std::chrono::duration<LONGLONG, std::ratio_multiply<std::hecto, std::nano>>
cpu_time
:execution_times
:thread_times
andprocess_times
: aliases forexecution_times
Functions
The following functions are defined:
Returns the
thread_times
for the thread identified by thethread
handle. Defaults to the current thread.Wraps
GetThreadTimes
, handling errors according toErrorPolicy
.ErrorPolicy
defaults toerr_exception_policy
when calling the non-template overloadReturns the CPU time usage specified by the
kind
argument for the thread identified by thethread
handle. Defaults to the total (kernel + user) CPU time of the current thread.Wraps
GetThreadTimes
, handling errors according toErrorPolicy
.ErrorPolicy
defaults toerr_exception_policy
when calling the non-template overloadReturns the
process_times
for the process identified by theprocess
handle. Defaults to the current process.Wraps
GetProcessTimes
, handling errors according toErrorPolicy
.ErrorPolicy
defaults toerr_exception_policy
when calling the non-template overloadReturns the CPU time usage specified by the
kind
argument for the process identified by theprocess
handle. Defaults to the total (kernel + user) CPU time of the current process.Wraps
GetProcessTimes
, handling errors according toErrorPolicy
.ErrorPolicy
defaults toerr_exception_policy
when calling the non-template overloadClocks
The following Clocks are defined:
current_thread_cpu_time_clock
:rep
,period
: same ascpu_time_duration
duration
:cpu_time_duration
now<ErrorPolicy>
: wrapsget_thread_cpu_time
with default arguments and the specifiedErrorPolicy
, returning the total CPU time used by the current threadnow
: wrapsnow
, passingerr_exception_policy
as itsErrorPolicy
current_process_cpu_time_clock
:rep
,period
: same ascpu_time_duration
duration
:cpu_time_duration
now<ErrorPolicy>
: wrapsget_process_cpu_time
with default arguments and the specifiedErrorPolicy
, returning the total CPU time used by the current processnow
: wrapsnow
, passingerr_exception_policy
as itsErrorPolicy
Caveats
Only error policies that interrupt execution (e.g.
err_failfast_policy
,err_exception_policy
) are meaningfully supported. Policies that return error codes will be effectively ignored, and errors will cause undefined values to be returnedCPU time clocks were only defined for the current process and current thread, so that they could be stateless classes. The Clock concept doesn't mandate statelessness, but it's unclear to me how statefulness conceptually interacts with
time_point
CPU time clocks were only defined for total time, because I couldn't think of a good, unambiguous API for passing a
cpu_time
argument