In ntmisc.c, we have void _PR_MD_INTERVAL_INIT() { LARGE_INTEGER count; if (QueryPerformanceFrequency(&count)) { while(count.LowPart > PR_INTERVAL_MAX) { count.LowPart >>= 1; _nt_bitShift++; _nt_highMask = (_nt_highMask << 1)+1; } _nt_ticksPerSec = count.LowPart; PR_ASSERT(_nt_ticksPerSec > PR_INTERVAL_MIN); } else _nt_ticksPerSec = -1; } Note that the code assumes count.HighPart is 0 and ignores it. count.LowPart is an unsigned 32-bit integer (DWORD). On some computers, the result returned by QueryPerformanceFrequency will be the clock speed of the CPU, and our code will break if the processor runs faster than 2^32 Hz, or ~ 4 GHz.

Mike, I'd appreciate it if you could review this patch. With this patch, NSPR no longer ignores the high 32 bits of the performance counter frequency. I also cleaned up the code a little bit. 1. I found that _nt_highMask is not really necessary and deleted it. 2. I replaced the multiple-step computation of the current time stamp into a single arithmetic expression in _PR_MD_GET_INTERVAL. I don't know why the original code does the shifting and addition in separate statements and reuses count.LowPart to store the intermediate and final results. I think combining these statements into one expression makes the code easier to understand.

Please use this patch instead. The previous patch was generated from an unreleased version of the file.

I tested this code on my machine, which has a frequency of 3579545ticks/sec (less than 32 bits). It tests fine in that case and does not differ from the original implementation. But I don't have a higher frequency clock to test with here, so I can't test that case. Visual inspection looks right. To be honest, I wonder if we aren't over-engineering here. Do we really need a sub-millisecond timer? Maybe just obliterate all this stuff and use GetTickCount() (units in milliseconds, 10ms resolution). There are enough platform-specific warnings in the MSFT documentation that I don't really trust QueryPerformanceCounter() to work in a portable way (MP issues, clock speed issues, HW issues). I'm pretty sure that attempting to use it will have future incompatibilities which we can probably avoid by using a more supported call.

On current processors we need to patch the NSPR library to artificially make the performance counter frequency greater than 4 GHz. The patches will be attached next.

Apply this patch to mozilla/nsprpub/pr/src/md/windows/ntinrval.c, rev. 3.8. Adjust the value of SHIFT until 'freq' is more than 32 bits. Then run the test program against the modified NSPR library. You'll see that the elapsed time is larger than 1000 ms (the expected value).

Apply this patch to mozilla/nsprpub/pr/src/md/windows/ntinrval.c, rev. 3.10. Adjust the value of SHIFT until 'freq' is more than 32 bits. (This step actually requires running the test program, which prints the value of 'freq'.) Then run the test program against the modified NSPR library. You'll see that the elapsed time is 1000 ms (the expected value).

Mike, Thank you for the code review. I've checked in the patch. I agree that QueryPerformanceCounter() has many problems and I will consider using GetTickCount(). The purpose of this bug is to document a potential problem of our code, just in case someone will install old NSPR releases (<= 4.2.2) on a 4GHz machine. I hope we can track down the causes of the problems reported in the original bug about PRIntervalTime (bug 115865) before making the decision to switch from QueryPerformanceCounter() to GetTickCount(). In the meantime I want to fix this known bug in our code.