win: use performance counter instead of GetTickCount for uptime #326

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

mscdex commented Feb 24, 2012

No description provided.

Contributor

mscdex commented Feb 24, 2012

Err... I somehow overlooked the fact that someone had already submitted a pull request a few months ago that does something similar. Oh well.

mscdex referenced this pull request Mar 2, 2012

Closed

win: implement cpu times #327

Member

piscisaureus commented Mar 7, 2012

I agree that this needs to be done. However @indutny also did a pull request (#203) and by the look of it his patch is closer to being acceptable than yours.

Member

piscisaureus commented Mar 7, 2012

@mscdex Ok I will take your patch if you fix the issues.

@piscisaureus piscisaureus commented on the diff Mar 7, 2012

src/win/util.c
+ uv_fatal_error(ERROR_OUTOFMEMORY, "malloc");
+ }
+ getSize = dataSize;
+
+ lError = RegQueryValueEx(HKEY_PERFORMANCE_DATA, "2", NULL, NULL,
+ (BYTE*)pPerfData, &getSize);
+ if (lError != ERROR_SUCCESS && getSize > 0) {
+ if (wcsncmp(pPerfData->Signature, "PERF", 4) == 0) {
+ break;
+ }
+ } else if (lError != ERROR_SUCCESS && lError != ERROR_MORE_DATA) {
+ err = uv__new_sys_error(GetLastError());
+ goto done;
+ }
+
+ dataSize += 1024;
@piscisaureus

piscisaureus Mar 7, 2012

Member

There is no need to spin to figure out the data size you need. From msdn:

If the buffer specified by lpData parameter is not large enough to hold the data, the function returns ERROR_MORE_DATA and stores the required buffer size in the variable pointed to by lpcbData. In this case, the contents of the lpData buffer are undefined.

If lpData is NULL, and lpcbData is non-NULL, the function returns ERROR_SUCCESS and stores the size of the data, in bytes, in the variable pointed to by lpcbData. This enables an application to determine the best way to allocate a buffer for the value's data.

@mscdex

mscdex Mar 7, 2012

Contributor

Right after that it also says:

If hKey specifies HKEY_PERFORMANCE_DATA and the lpData buffer is not large enough to contain all of the returned data, RegQueryValueEx returns ERROR_MORE_DATA and the value returned through the lpcbData parameter is undefined. This is because the size of the performance data can change from one call to the next. In this case, you must increase the buffer size and call RegQueryValueEx again passing the updated buffer size in the lpcbData parameter. Repeat this until the function succeeds. You need to maintain a separate variable to keep track of the buffer size, because the value returned by lpcbData is unpredictable.

@piscisaureus

piscisaureus Mar 7, 2012

Member

Right - ok, so I agree with your approach. Although I wouldn't mind starting off with a somewhat bigger buffer.

@mscdex

mscdex Mar 7, 2012

Contributor

What size do you suggest? 4kb?

@piscisaureus

piscisaureus Mar 7, 2012

Member

Yeah, that would be nice.

@piscisaureus piscisaureus and 1 other commented on an outdated diff Mar 7, 2012

src/win/util.c
+ UINT i;
+ BYTE *pData;
+
+ *uptime = 0;
+
+ while (lError == ERROR_MORE_DATA) {
+ if (pPerfData) {
+ free(pPerfData);
+ }
+ pPerfData = (PERF_DATA_BLOCK*)malloc(dataSize);
+ if (!pPerfData) {
+ uv_fatal_error(ERROR_OUTOFMEMORY, "malloc");
+ }
+ getSize = dataSize;
+
+ lError = RegQueryValueEx(HKEY_PERFORMANCE_DATA, "2", NULL, NULL,
@piscisaureus

piscisaureus Mar 7, 2012

Member

Be explicit about the character type you want (use RegQueryValueExA or RegQueryValueExW)

@mscdex

mscdex Mar 7, 2012

Contributor

I just used RegQueryValueEx because that's what uv_cpu_info was already using for reading cpu speed and name info (cares uses RegQueryValueEx also). I'm not sure if the unicode version is preferred over the ansi version or vice versa for this case.

@piscisaureus

piscisaureus Mar 7, 2012

Member

So - uv_cpu_info is doing it wrong. The problem is that RegQueryValueEx maps to either RegQueryValueExA or RegQueryValueExW, depending on the setting of the _UNICODE directive. If you want to use RegQueryValueEx you should also encapsulate string constants in _TEXT. But don't. Just use the wide-character variant (RegQueryValueExW).

@piscisaureus piscisaureus and 1 other commented on an outdated diff Mar 7, 2012

src/win/util.c
+ }
+ } else if (lError != ERROR_SUCCESS && lError != ERROR_MORE_DATA) {
+ err = uv__new_sys_error(GetLastError());
+ goto done;
+ }
+
+ dataSize += 1024;
+ }
+
+ RegCloseKey(HKEY_PERFORMANCE_DATA);
+
+ pObj = (PERF_OBJECT_TYPE*)((BYTE*)pPerfData + pPerfData->HeaderLength);
+ pDef = (PERF_COUNTER_DEFINITION*)((BYTE*)pObj + pObj->HeaderLength);
+
+ for (i = 0; i < pObj->NumCounters; ++i) {
+ if (pDef->CounterNameTitleIndex == 674) {
@piscisaureus

piscisaureus Mar 7, 2012

Member

Is 674 a fixed number associated with uptime?

@mscdex

mscdex Mar 7, 2012

Contributor

Yep

@piscisaureus

piscisaureus Mar 7, 2012

Member

Just curious, is this documented somewhere? Or is it documented somewhere that title indices are "stable" and never change?

@mscdex

mscdex Mar 7, 2012

Contributor

There isn't any official documentation of it that I could find. However, libraries like sigar, code samples from the msdn blog, and just about every other Windows uptime snippets on the internet that use performance counters instead of GetTickCount, all use 2 for the "System" counter type and 674 for the "System Up Time" counter id. Also, there was a post on microsoft.public.win32.programmer.kernel back in 1998 that mentions using these two constant values for retrieving the system up time :)

FWIW, the built-in counter types, ids, and names can be seen in: HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Perflib<LangID>\Counter

@piscisaureus piscisaureus commented on an outdated diff Mar 7, 2012

src/win/util.c
@@ -385,8 +385,70 @@ uv_err_t uv_resident_set_memory(size_t* rss) {
uv_err_t uv_uptime(double* uptime) {
- *uptime = (double)GetTickCount()/1000.0;
- return uv_ok_;
+ uv_err_t err;
+ PERF_DATA_BLOCK *pPerfData = NULL;
@piscisaureus

piscisaureus Mar 7, 2012

Member

Please don't use hungarian notation.

@piscisaureus piscisaureus commented on an outdated diff Mar 7, 2012

src/win/util.c
@@ -385,8 +385,70 @@ uv_err_t uv_resident_set_memory(size_t* rss) {
uv_err_t uv_uptime(double* uptime) {
- *uptime = (double)GetTickCount()/1000.0;
- return uv_ok_;
+ uv_err_t err;
+ PERF_DATA_BLOCK *pPerfData = NULL;
+ PERF_OBJECT_TYPE *pObj;
+ PERF_COUNTER_DEFINITION *pDef;
+ PERF_COUNTER_DEFINITION *pDefUptime = NULL;
+ DWORD dataSize = 1024;
+ DWORD getSize;
+ LONG lError = ERROR_MORE_DATA;
+ unsigned __int64 upsec;
@piscisaureus

piscisaureus Mar 7, 2012

Member

use uint64_t

@piscisaureus piscisaureus commented on an outdated diff Mar 7, 2012

src/win/util.c
@@ -385,8 +385,70 @@ uv_err_t uv_resident_set_memory(size_t* rss) {
uv_err_t uv_uptime(double* uptime) {
- *uptime = (double)GetTickCount()/1000.0;
- return uv_ok_;
+ uv_err_t err;
+ PERF_DATA_BLOCK *pPerfData = NULL;
+ PERF_OBJECT_TYPE *pObj;
+ PERF_COUNTER_DEFINITION *pDef;
+ PERF_COUNTER_DEFINITION *pDefUptime = NULL;
+ DWORD dataSize = 1024;
+ DWORD getSize;
+ LONG lError = ERROR_MORE_DATA;
+ unsigned __int64 upsec;
+ UINT i;
@piscisaureus

piscisaureus Mar 7, 2012

Member

use windows types only for values passed to / return from win32 api calls.

@piscisaureus piscisaureus commented on an outdated diff Mar 7, 2012

src/win/util.c
+
+ pObj = (PERF_OBJECT_TYPE*)((BYTE*)pPerfData + pPerfData->HeaderLength);
+ pDef = (PERF_COUNTER_DEFINITION*)((BYTE*)pObj + pObj->HeaderLength);
+
+ for (i = 0; i < pObj->NumCounters; ++i) {
+ if (pDef->CounterNameTitleIndex == 674) {
+ pDefUptime = pDef;
+ break;
+ }
+ pDef = (PERF_COUNTER_DEFINITION*)((BYTE*)pDef + pDef->ByteLength);
+ }
+
+ pData = (BYTE*)pObj + pObj->DefinitionLength;
+ pData += pDefUptime->CounterOffset;
+
+ memcpy(&upsec, pData, 8);
@piscisaureus

piscisaureus Mar 7, 2012

Member

You really need to memcpy here? What about casting pData to int64_t*?

Member

piscisaureus commented Apr 1, 2012

Thanks, Brian. Landed in 9d3c000 and 442aa1f

Member

piscisaureus commented Apr 12, 2012

@mscdex Why did we do this again? Was it only because GetTickCount could wrap, or where there other reasons?

Contributor

mscdex commented Apr 12, 2012

Yeah, GetTickCount does wrap after 49.7 days. Also one other reason for this specific solution was that GetTickCount64 is Vista+ only.

net147 commented Apr 17, 2013

A downside of this is that the performance counter is affected by date/time changes on Windows XP (but not Windows Vista and later). See joyent/node#5304.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment