On 12/05/2014 08:03 PM, David Rowley wrote:
> On 2 December 2014 at 15:36, Craig Ringer <cr...@2ndquadrant.com
> <mailto:cr...@2ndquadrant.com>> wrote:
> 
>     On 12/01/2014 09:51 PM, Marco Nenciarini wrote:
>     > I think this is a leftover, as you don't use elog afterwards.
> 
>     Good catch, fixed.
> 
> I've looked over this again and tested it on a windows 8.1 machine. I
> cannot find any problems
> 
> The only comments about the code I have would maybe be to use some
> constants like:
> 
> #define FILETIME_PER_SEC10000000L
> #define FILETIME_PER_USEC10

[snip]

> I'll leave it up to the committer to decide if it's better with or
> without the attached patch.

I think it's more readable, and that's pretty much always a good thing.

Patches with your changes attached.

I used FILETIME_UNITS_PER_SEC though.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 9c4e7f5539f0d518b0fe94d12bc562d95967f6a6 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 12 Sep 2014 12:41:35 +0800
Subject: [PATCH 1/2] Use GetSystemTimeAsFileTime directly in win32
 gettimeofday
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PostgreSQL was calling GetSystemTime followed by SystemTimeToFileTime in the
win32 port gettimeofday function. This is not necessary and limits the reported
precision to the 1ms granularity that the SYSTEMTIME struct can represent. By
using GetSystemTimeAsFileTime we avoid unnecessary conversions and capture
timestamps at 100ns granularity, which is then rounded to 1µs granularity for
storage in a PostgreSQL timestamp.

On most Windows systems this change will actually have no significant effect on
timestamp resolution as the system timer tick is typically between 1ms and 15ms
depending on what timer resolution currently running applications have
requested. You can check this with clockres.exe from sysinternals. Despite the
platform limiation this change still permits capture of finer timestamps where
the system is capable of producing them and it gets rid of an unnecessary
syscall.

The higher resolution GetSystemTimePreciseAsFileTime call available on Windows
8 and Windows Server 2012 has the same interface as GetSystemTimeAsFileTime, so
switching to GetSystemTimeAsFileTime makes it easier to use the Precise variant
later.
---
 src/port/gettimeofday.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/port/gettimeofday.c b/src/port/gettimeofday.c
index 75a9199..ecc0b4d 100644
--- a/src/port/gettimeofday.c
+++ b/src/port/gettimeofday.c
@@ -31,10 +31,17 @@
 #include <sys/time.h>
 
 
-/* FILETIME of Jan 1 1970 00:00:00. */
+/* FILETIME of Jan 1 1970 00:00:00, the PostgreSQL epoch */
 static const unsigned __int64 epoch = UINT64CONST(116444736000000000);
 
 /*
+ * FILETIME represents the number of 100-nanosecond intervals since
+ * January 1, 1601 (UTC).
+ */
+#define FILETIME_UNITS_PER_SEC	10000000L
+#define FILETIME_UNITS_PER_USEC	10
+
+/*
  * timezone information is stored outside the kernel so tzp isn't used anymore.
  *
  * Note: this function is not for Win32 high precision timing purpose. See
@@ -44,16 +51,15 @@ int
 gettimeofday(struct timeval * tp, struct timezone * tzp)
 {
 	FILETIME	file_time;
-	SYSTEMTIME	system_time;
 	ULARGE_INTEGER ularge;
 
-	GetSystemTime(&system_time);
-	SystemTimeToFileTime(&system_time, &file_time);
+	GetSystemTimeAsFileTime(&file_time);
 	ularge.LowPart = file_time.dwLowDateTime;
 	ularge.HighPart = file_time.dwHighDateTime;
 
-	tp->tv_sec = (long) ((ularge.QuadPart - epoch) / 10000000L);
-	tp->tv_usec = (long) (system_time.wMilliseconds * 1000);
+	tp->tv_sec = (long) ((ularge.QuadPart - epoch) / FILETIME_UNITS_PER_SEC);
+	tp->tv_usec = (long) (((ularge.QuadPart - epoch) % FILETIME_UNITS_PER_SEC)
+		/ FILETIME_UNITS_PER_USEC);
 
 	return 0;
 }
-- 
1.9.3

From 30cfd48f05bee68602c3088f49c1e30c2251a84f Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Thu, 18 Sep 2014 23:02:14 +0800
Subject: [PATCH 2/2] On Windows, use GetSystemTimePreciseAsFileTime when
 available

PostgreSQL on Windows 8 or Windows Server 2012 will now obtain
high-resolution timestamps by dynamically loading the the
GetSystemTimePreciseAsFileTime function. It'll fall back to
GetSystemTimeAsFileTime if the higher precision variant isn't found,
so the same binaries without problems on older Windows releases.

No attempt is made to detect the Windows version. Only the presence or
absence of the desired function is considered.
---
 src/backend/main/main.c |  6 ++++++
 src/include/port.h      |  2 ++
 src/port/gettimeofday.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index c51b391..73c30c5 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -260,6 +260,12 @@ startup_hacks(const char *progname)
 
 		/* In case of general protection fault, don't show GUI popup box */
 		SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
+
+#ifndef HAVE_GETTIMEOFDAY
+		/* Figure out which syscall to use to capture timestamp information */
+		init_win32_gettimeofday();
+#endif
+
 	}
 #endif   /* WIN32 */
 
diff --git a/src/include/port.h b/src/include/port.h
index 94a0e2f..58677ec 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -328,6 +328,8 @@ extern FILE *pgwin32_popen(const char *command, const char *type);
 #ifndef HAVE_GETTIMEOFDAY
 /* Last parameter not used */
 extern int	gettimeofday(struct timeval * tp, struct timezone * tzp);
+/* On windows we need to call some backend start setup for accurate timing */
+extern void init_win32_gettimeofday(void);
 #endif
 #else							/* !WIN32 */
 
diff --git a/src/port/gettimeofday.c b/src/port/gettimeofday.c
index ecc0b4d..eabf161 100644
--- a/src/port/gettimeofday.c
+++ b/src/port/gettimeofday.c
@@ -42,9 +42,58 @@ static const unsigned __int64 epoch = UINT64CONST(116444736000000000);
 #define FILETIME_UNITS_PER_USEC	10
 
 /*
+ * Both GetSystemTimeAsFileTime and GetSystemTimePreciseAsFileTime share a
+ * signature, so we can just store a pointer to whichever we find. This
+ * is the pointer's type.
+ */
+typedef VOID (WINAPI *PgGetSystemTimeFn)(LPFILETIME);
+
+/* Storage for the function we pick at runtime */
+static PgGetSystemTimeFn pg_get_system_time = NULL;
+
+/*
+ * During backend startup, determine if GetSystemTimePreciseAsFileTime is
+ * available and use it; if not, fall back to GetSystemTimeAsFileTime.
+ */
+void
+init_win32_gettimeofday(void)
+{
+	/*
+	 * Because it's guaranteed that kernel32.dll will be linked into our
+	 * address space already, we don't need to LoadLibrary it and worry about
+	 * closing it afterwards, so we're not using Pg's dlopen/dlsym() wrapper.
+	 *
+	 * We'll just look up the address of GetSystemTimePreciseAsFileTime if
+	 * present.
+	 *
+	 * While we could look up the Windows version and skip this on Windows
+	 * versions below Windows 8 / Windows Server 2012 there isn't much point,
+	 * and determining the windows version is its self somewhat Windows version
+	 * and development SDK specific...
+	 */
+	pg_get_system_time = (PgGetSystemTimeFn) GetProcAddress(
+			GetModuleHandle(TEXT("kernel32.dll")),
+				"GetSystemTimePreciseAsFileTime");
+	if (pg_get_system_time == NULL)
+	{
+		/*
+		 * The expected error from GetLastError() is ERROR_PROC_NOT_FOUND, if
+		 * the function isn't present. No other error should occur.
+		 *
+		 * It's too early in startup to elog(...) if we get some unexpected
+		 * error, and not serious enough to warrant a fprintf to stderr about
+		 * it or save the error and report it later. So silently fall back to
+		 * GetSystemTimeAsFileTime irrespective of why the failure occurred.
+		 */
+		pg_get_system_time = &GetSystemTimeAsFileTime;
+	}
+
+}
+
+/*
  * timezone information is stored outside the kernel so tzp isn't used anymore.
  *
- * Note: this function is not for Win32 high precision timing purpose. See
+ * Note: this function is not for Win32 high precision timing purposes. See
  * elapsed_time().
  */
 int
@@ -53,7 +102,7 @@ gettimeofday(struct timeval * tp, struct timezone * tzp)
 	FILETIME	file_time;
 	ULARGE_INTEGER ularge;
 
-	GetSystemTimeAsFileTime(&file_time);
+	(*pg_get_system_time)(&file_time);
 	ularge.LowPart = file_time.dwLowDateTime;
 	ularge.HighPart = file_time.dwHighDateTime;
 
-- 
1.9.3

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to