On 10/22/2014 04:12 PM, David Rowley wrote:
> I was just having a quick look at this with the view of testing it on a
> windows 8 machine.
Thankyou. I really appreciate your taking the time to do this, as one of
the barriers to getting Windows-specific patches accepted is that
usually people just don't want to review Windows patches.
I see you've already linked it on the commitfest too, so thanks again.
I'll follow up with a fixed patch and my test code shortly. I'm at PGEU
in Madrid so things are a bit chaotic, but I can make some time.
> +pg_get_system_time = (PgGetSystemTimeFn) GetProcAddress(
> +GetModuleHandle(TEXT("kernel32.dll")),
> +"GetSystemRimePreciseAsFileTime");
>
>
> "Rime", not doubt is meant to be "Time".
Hm.
I must've somehow managed to attach and post the earlier version of the
patch before I fixed a few issues and tested it, because I've compiled
and tested this feature on Win2k12.
Apparently just not the version I published.
Thanks for catching that. I'll fix it up.
> +elog(DEBUG1, "GetProcAddress(\"GetSystemRimePreciseAsFileTime\") on
> kernel32.dll failed with error code %d not expected
> ERROR_PROC_NOT_FOUND(127)", errcode);
>
> But I don't think you'll be able to elog quite that early. I tested by
> getting rid of the if (errcode != ERROR_PROC_NOT_FOUND) test and I get:
>
> D:\Postgres\install\bin>postgres -D ..\data
> error occurred at src\port\gettimeofday.c:87 before error message
> processing is available
Thankyou. I didn't consider that logging wouldn't be available there.
This case shouldn't really happen.
> Perhaps we needn't bother with this debug message? Either that you'll
> probably need to cache the error code and do something when the logger
> is initialised.
In a "shouldn't happen" case like this I think it'll be OK to just print
to stderr.
> I see:
> test=# select current_timestamp;
> NOTICE: 4
> NOTICE: GetSystemTimePreciseAsFileTime
>
> Which indicates that this is quite a precise timer.
Great.
Because I was testing on AWS I wasn't getting results that fine, but the
kind of granularity you're seeing is consistent with what I get on my
Linux laptop.
> Whereas if I add a quick hack into init_win32_gettimeofday() so that it
> always chooses GetSystemTimeAsFileTime() I see:
>
> test=# select current_timestamp;
> NOTICE: 9953
> NOTICE: GetSystemTimeAsFileTime
I'll publish the test code I was using too. I was doing it from SQL
level with no code changes other than the ones required for timestamp
precision.
-- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services