Re: [Windows,PATCH] Use faster, higher precision timer API

On Thu, Oct 23, 2014 at 1:27 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:

> Here's an updated patch addressing David's points.
>
> I haven't had a chance to test it yet, on win2k8 or win2k12 due to
> pgconf.eu .
>
>
Hi Craig, thanks for the fast turnaround.

I've just had a look over the patch again:

+ DWORD errcode = GetLastError();
+ Assert(errcode == ERROR_PROC_NOT_FOUND);

I'm not a big fan of this. It seems quite strange to be using Assert in
this way. I'd rather see any error just silently fall back
on GetSystemTimeAsFileTime() instead of this. I had originally assumed that
you stuck the debug log in there so that people would have some sort of way
of finding out if their system is using GetSystemTimePreciseAsFileTime() or
GetSystemTimeAsFileTime(), the assert's not really doing this. I'd vote
for, either removing this assert or sticking some elog DEBUG1 sometime
after the logger has started. Perhaps just a test like:

if (pg_get_system_time == &GetSystemTimeAsFileTime)
elog(DEBUG1, "gettimeofday is using GetSystemTimeAsFileTime()");
else
elog(DEBUG1, "gettimeofday is using GetSystemTimePreciseAsFileTime()");

But perhaps it's not worth the trouble.

Also if you decide to get rid of the elog, probably should also remove the
include of elog.h that you've added. Or if you disagree with my comment on
the Assert() you'll need to include the proper header for that. The
compiler is currently giving a warning about that.

Regards

David Rowley