#1600 [WIN32] SetWaitableTimer() doesn't use QueryPerformanceCounter()

[Carlo Bramini]

In my opinion, there is a critical bug inside vice/src/ticks.c for Windows.
Inside tick_sleep(), the parameter "sleep_ticks" is scaled according to the frequency of the performance counter, but this is wrong. The function SetWaitableTimer() configures a previously allocated timer to be signaled in 100 nanosecond intervals:

https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-setwaitabletimer

On my PC, the current code worked for pure luck, because the performance counter returned frequency of 10MHz, that it is exactly the frequency for getting the 10x factor we need to be multiplied to TICK_PER_SECOND, which is fixed to 1MHz, for getting the 100ns granularity.
But other versions of Windows do not:

https://answers.microsoft.com/en-us/windows/forum/all/queryperformancefrequency-qpc-is-10-mhz-since/d0fb399d-5dfd-4a7a-af5f-220751953ad0

However, according to the guidelines from Microsoft, we should never assume that QueryPerformanceFrequency() returns a particular frequency:

https://docs.microsoft.com/en-us/windows/win32/sysinfo/acquiring-high-resolution-time-stamps

This can also be the cause of the troubles you got with timing and synchronization on Windows according to what you wrote into vice/src/ticks.h.

There are two possible solutions here:
1) tick_per_second() can return timer_frequency instead of TICK_PER_SECOND, tick_now() just returns the value you got from QueryPerformanceCounter() without scaling and tick_sleep() just needs to scale its parameter to 100ns ticks according to timer_frequency.
2) leave tick_per_second() and tick_now() as they are and scale the parameter of tick_sleep() to 100ns according to TICK_PER_SECOND.

Solution n.1 seems to provide more flexibility, while solution n.2 required less changes. You can decide to use the one you like more and, if you want, I can provide that patch.

There is also a less important issue here, because according to the coding guide lines, you cannot declare variables "where you want", but just on top of an {...} block. A patch could also include these fixes.

[dqh]

Thanks for the report - fixed in r41066

[Querino]

were only newer versions affected by this?

i never had a problem using win 7

[dqh]

although it was wrong, the way sleep is used in vice it didnt really cause a problem. So it wasn't critical in any practical sense.

[Querino]

got it, thanks.