[libcxx-commits] [PATCH] D104987: Use GetSystemTimePreciseAsFileTime() if available

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D104987#2855606 <https://reviews.llvm.org/D104987#2855606>, @amccarth wrote:

> tl;dr:  I understand the desire to avoid even tiny synchronization costs when trying to read a high-precision timer, but I'm uncomfortable with the data race.  I suggest initializing the static function pointer under the locking provided by the compiler.
>
> The simplest option would be to initialize it where it's defined.  I would move the code that finds the address to a separate function, say `GetGetSysTimeProcAddress`, and then simplify this code to:
>
>   static GetSysTime_t getSysTime_p = GetGetSysTimeProcAddress();
>   getSysTime_p(&ft);

Oh, that's great, I had no idea that this actually ends up synchronized. That's very neat. And the overhead once initialized seems miniscule.

Yeah such a setup sounds good to me, and clearly better than the other options listed.

> Minor note:  `GetModuleHandleW` might be a little faster than `GetModuleHandleA`.  I believe the `-A` version will do the equivalent of a call to `MultiByteToWide`, which you could avoid by calling `-W` explicitly and adding an `L` prefix to the string literal.  (That's just for the module name.  The proc names are always "ANSI".)

On this note, note that for store apps, we can't use `GetModuleHandle` at all. So if `_LIBCPP_WINDOWS_STORE_APP` is defined, we should stick to static compile time switching between the functions.

FWIW, for static switching of the function, it seems like the more precise definition for when it's available is more like `#if (_WIN32_WINNT >= _WIN32_WINNT_WIN8 && WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)) || (_WIN32_WINNT >= _WIN32_WINNT_WIN10)`. If `_LIBCPP_WINDOWS_STORE_APP` is defined, then `WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)` equals false. So if `_LIBCPP_WINDOWS_STORE_APP` is defined, we can use the newer function since `_WIN32_WINNT >= _WIN32_WINNT_WIN10`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104987/new/

https://reviews.llvm.org/D104987