[PROPOSAL] Platform-native resource usage stats for Windows

Started by Bryan Green3 months ago1 messages
#1Bryan Green
dbryan.green@gmail.com
1 attachment(s)

Hello,

I'd like to propose an enhancement to how PostgreSQL reports resource
usage statistics on Windows, building on a 2017 discussion initiated by
Justin Pryzby [1]/messages/by-id/20170610222026.GE18003@telsasoft.com.

I've been spending some time reviewing windows specific code in the
postgresql codebase. I've notice several "opportunities" for
enhancements and cleanup-- especially if we are only supporting Windows
10+. One of the areas I noticed was getrusage().

The current Windows getrusage() shim in src/port/getrusage.c only
reports CPU times. Everything else is zero. Windows has better
performance APIs than Unix getrusage() in several areas - we're just not
using them.

Back in 2017, Justin Pryzby proposed exposing ru_maxrss [1]/messages/by-id/20170610222026.GE18003@telsasoft.com, and Robert
Haas suggested that if we're going to customize by platform, we should
show only meaningful values per platform [2]https://www.mail-archive.com/pgsql-hackers@postgresql.org/msg316929.html. Nobody followed through
with a patch.

Problem

Windows administrators get this:
! 0/0 [0/0] filesystem blocks in/out
! 0/39 [0/1281] page faults/reclaims, 0 [0] swaps

The zeros are uninformative. Windows has the data - I/O byte counts,
detailed memory stats, handle counts - but we're trying to jam it into
struct rusage where it doesn't fit.

I propose we let each platform report what it actually measures, using
native APIs:

Windows: GetProcessMemoryInfo, GetProcessIoCounters, QueryProcessCycleTime
Linux/BSD: Keep using getrusage, maybe expose more fields later

This means platform-specific output formats, but that's better than
reporting zeros. This follows the precedent Robert Haas suggested in
2017 [2]https://www.mail-archive.com/pgsql-hackers@postgresql.org/msg316929.html: "if we're going to go to the trouble of customizing this code
by platform, we ought to get rid of values that are meaningless on that
platform."

We already have extensive platform-specific code in
src/backend/port/win32/. This follows the same pattern.

I've included a rough proof-of-concept. It:

(1) Adds PgWin32ResourceUsage extension to PGRUsage
(2) Implements Windows-native collection via additions to pg_rusage_init()
(3) Shows actual I/O bytes, memory details, handle/thread counts
(4) Leaves Unix paths completely untouched

On Windows you'd see:
! Memory: 45232 KB working set (48192 KB peak), 38416 KB private
! I/O: 2457600 bytes read (147 ops), 819200 bytes written (43 ops)
! Resources: 247 handles, 8 threads

Before I invest time in a full patch:

(1) Is there support for this general approach of platform-specific
resource reporting?
(2) Should this extend to non-Windows Unix platforms as well (e.g.,
exposing more Linux-specific fields)?
(3) Any concerns about diverging output formats across platforms?
(4) Would you prefer all platforms to show only common fields, or
platform-native output?

I believe this aligns with PostgreSQL's existing philosophy - we already
have extensive platform-specific code in src/backend/port/win32/, and
this would be a natural extension of that approach.

I'm happy to implement this if there's community interest. Feedback welcome!

[1]: /messages/by-id/20170610222026.GE18003@telsasoft.com
/messages/by-id/20170610222026.GE18003@telsasoft.com
[2]: https://www.mail-archive.com/pgsql-hackers@postgresql.org/msg316929.html

BG

Attachments:

pg_rusage_win32_poc.patchtext/plain; charset=UTF-8; name=pg_rusage_win32_poc.patchDownload
diff --git a/src/include/utils/pg_rusage.h b/src/include/utils/pg_rusage.h
index 1234567..abcdefg 100644
--- a/src/include/utils/pg_rusage.h
+++ b/src/include/utils/pg_rusage.h
@@ -15,10 +15,31 @@
 
 #include <sys/resource.h>
 
+#ifdef WIN32
+typedef struct PgWin32ResourceUsage
+{
+	uint64		io_read_bytes;
+	uint64		io_write_bytes;
+	uint64		io_other_bytes;
+	uint32		io_read_ops;
+	uint32		io_write_ops;
+	size_t		working_set_size;
+	size_t		peak_working_set_size;
+	size_t		private_bytes;
+	uint32		page_fault_count;
+	uint32		handle_count;
+	uint32		thread_count;
+} PgWin32ResourceUsage;
+#endif
 
 typedef struct PGRUsage
 {
 	struct rusage ru;
 	struct timeval tv;
+#ifdef WIN32
+	PgWin32ResourceUsage win32;
+#endif
 } PGRUsage;
 
 
diff --git a/src/backend/utils/misc/pg_rusage.c b/src/backend/utils/misc/pg_rusage.c
index 2345678..bcdefgh 100644
--- a/src/backend/utils/misc/pg_rusage.c
+++ b/src/backend/utils/misc/pg_rusage.c
@@ -18,6 +18,12 @@
 
 #include "utils/pg_rusage.h"
 
+#ifdef WIN32
+#include <windows.h>
+#include <psapi.h>
+#include <tlhelp32.h>
+#endif
+
 
 /*
  * Initialize usage snapshot.
@@ -26,8 +32,87 @@ void
 pg_rusage_init(PGRUsage *ru0)
 {
 	getrusage(RUSAGE_SELF, &ru0->ru);
 	gettimeofday(&ru0->tv, NULL);
+
+#ifdef WIN32
+	{
+		HANDLE hProcess = GetCurrentProcess();
+		PROCESS_MEMORY_COUNTERS_EX memCounters;
+		IO_COUNTERS ioCounters;
+		
+		memset(&ru0->win32, 0, sizeof(PgWin32ResourceUsage));
+		
+		/* Get memory statistics */
+		memset(&memCounters, 0, sizeof(memCounters));
+		memCounters.cb = sizeof(memCounters);
+		if (GetProcessMemoryInfo(hProcess,
+								 (PROCESS_MEMORY_COUNTERS *)&memCounters,
+								 sizeof(memCounters)))
+		{
+			ru0->win32.working_set_size = memCounters.WorkingSetSize;
+			ru0->win32.peak_working_set_size = memCounters.PeakWorkingSetSize;
+			ru0->win32.private_bytes = memCounters.PrivateUsage;
+			ru0->win32.page_fault_count = memCounters.PageFaultCount;
+		}
+		
+		/* Get I/O statistics */
+		memset(&ioCounters, 0, sizeof(ioCounters));
+		if (GetProcessIoCounters(hProcess, &ioCounters))
+		{
+			ru0->win32.io_read_bytes = ioCounters.ReadTransferCount;
+			ru0->win32.io_write_bytes = ioCounters.WriteTransferCount;
+			ru0->win32.io_other_bytes = ioCounters.OtherTransferCount;
+			ru0->win32.io_read_ops = (uint32)ioCounters.ReadOperationCount;
+			ru0->win32.io_write_ops = (uint32)ioCounters.WriteOperationCount;
+		}
+		
+		/* Get handle count */
+		GetProcessHandleCount(hProcess, &ru0->win32.handle_count);
+		
+		/* Get thread count using toolhelp32 snapshot */
+		{
+			HANDLE hSnapshot;
+			THREADENTRY32 te32;
+			DWORD dwOwnerPID = GetCurrentProcessId();
+			
+			ru0->win32.thread_count = 0;
+			
+			hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
+			if (hSnapshot != INVALID_HANDLE_VALUE)
+			{
+				te32.dwSize = sizeof(THREADENTRY32);
+				
+				if (Thread32First(hSnapshot, &te32))
+				{
+					do
+					{
+						if (te32.th32OwnerProcessID == dwOwnerPID)
+							ru0->win32.thread_count++;
+					} while (Thread32Next(hSnapshot, &te32));
+				}
+				CloseHandle(hSnapshot);
+			}
+		}
+	}
+#endif
 }
 
 /*
@@ -83,6 +168,26 @@ pg_rusage_show(const PGRUsage *ru0)
 					   (long) (r.ru_oublock - ru0->ru.ru_oublock));
 	appendStringInfo(&str, _("!\t%ld/%ld [%ld/%ld] voluntary/involuntary context switches\n"),
 					 r.ru_nvcsw - ru0->ru.ru_nvcsw, r.ru_nivcsw - ru0->ru.ru_nivcsw,
 					 r.ru_nvcsw, r.ru_nivcsw);
+
+#ifdef WIN32
+	/* Show Windows-specific extended statistics */
+	appendStringInfo(&str, _("!\tWindows: %zu KB working set (%zu KB peak), %zu KB private\n"),
+					 r_win32.working_set_size / 1024,
+					 r_win32.peak_working_set_size / 1024,
+					 r_win32.private_bytes / 1024);
+	
+	appendStringInfo(&str, _("!\tI/O: %llu bytes read (%u ops), %llu write (%u ops), %llu other\n"),
+					 (unsigned long long)(r_win32.io_read_bytes - ru0->win32.io_read_bytes),
+					 r_win32.io_read_ops - ru0->win32.io_read_ops,
+					 (unsigned long long)(r_win32.io_write_bytes - ru0->win32.io_write_bytes),
+					 r_win32.io_write_ops - ru0->win32.io_write_ops,
+					 (unsigned long long)(r_win32.io_other_bytes - ru0->win32.io_other_bytes));
+	
+	appendStringInfo(&str, _("!\tResources: %u handles, %u threads, %u page faults\n"),
+					 r_win32.handle_count,
+					 r_win32.thread_count,
+					 r_win32.page_fault_count - ru0->win32.page_fault_count);
+#endif
 
 	return str.data;
 }
diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile
index 3456789..cdefghi 100644
--- a/src/backend/utils/misc/Makefile
+++ b/src/backend/utils/misc/Makefile
@@ -8,3 +8,7 @@ include $(top_builddir)/src/Makefile.global
 OBJS = \
 	guc.o \
 	pg_rusage.o
+
+ifeq ($(PORTNAME), win32)
+LIBS += -lpsapi
+endif