Disable `adjust_cpu_clock()` in rdtsc timer

[jwt27]

I was looking to find out how this code works, and see if maybe the gettimeofday() call could be replaced with something else.

https://github.com/gvanem/Watt-32/blob/5f72db8e5cfe8e7c701ecc958030b6118af93b6b/src/gettod.c#L363-L399

But it's not clear to me what it's trying to do. tick and last are always multiples of 5, so:

corr = abs ((int)last-(int)tick) / 8
     = abs (     10  -     15  ) / 8
     = 5 / 8
     = 0

The correction is always zero. Maybe it does something if you call it less often, say once every second, but that is not how it's used here. And even then I don't quite see how it relates BIOS ticks to rdtsc ticks.

But then at midnight:

corr = abs ((int)last-(int)tick) / 8
     = abs ( 1573035 -     0   ) / 8
     = 1573035 / 8
     = 196629

And it completely breaks.

So I think the safe option for now is to disable this code.

[gvanem]

But it's not clear to me what it's trying to do. tick and last are always multiples of 5, so:

I did not wrote that. But could have modified it.

But it seems to be a simple averaging filter to correct the errors from get_rdtsc and the initial value of clocks_per_usec. Ref. the jitter in your graph from issue #99. This is especially important on a multi-CPU system. Ref: win_get_rdtsc(). But you are on a single-CPU PC in MSDOS I guess?

corr = abs ((int)last-(int)tick) / 8

I fail to see where 8 comes from.

[jwt27]

But it's not clear to me what it's trying to do. tick and last are always multiples of 5, so:

I did not wrote that. But could have modified it.

If it was modified it would be useful to know what it originally looked like. Same thing happened with #100/#102. But we're missing all this history in git. Do you still happen to have an archive of old Wattcp/Watt32 versions?

But it seems to be a simple averaging filter to correct the errors from get_rdtsc and the initial value of clocks_per_usec. Ref. the jitter in your graph from issue #99. This is especially important on a multi-CPU system. Ref: win_get_rdtsc(). But you are on a single-CPU PC in MSDOS I guess?

I'm not very familiar with Windows programming. Can you read the BIOS clock there via PEEKL()? I thought that would be a DOS-only thing.

corr = abs ((int)last-(int)tick) / 8

I fail to see where 8 comes from.

Could it be that corr/clocks_per_usec was meant to be floating-point? Then it would add or subtract 0.625 from the cpu clock every 1375ms. But you could do that without tick/last. And then I still don't see the relationship between the correction amount and cpu cycles.

[gvanem]

Can you read the BIOS clock there via PEEKL()? I thought that would be a DOS-only thing.

Not at all. I imagined clocks_per_usec was also corrected on Windows too. Apparently not.

Do you still happen to have an archive of old Wattcp/Watt32 versions?

I've uploaded all the old .zip-files here: https://watt-32.net/old-stuff/

And using this watt-zipgrep.bat:

@echo off
setlocal
if %_cmdproc. == TCC. on break quit
set ZIPGREP=f:\ProgramFiler\Git-2\usr\bin\sh.exe f:\ProgramFiler\Git-2\usr\bin\zipgrep
for %f in (watt32s*.zip) do (
  echo zipgrepping in %f
  %ZIPGREP% "adjust_cpu_clock" %f src/*.c
)

found these:

zipgrepping in watt32s-v210-d1.zip
zipgrepping in watt32s-v220-d1.zip
zipgrepping in watt32s-v220-d2.zip
src/gettod.c:static long adjust_cpu_clocks (const struct timeval *tv)
src/gettod.c:    clocks_per_usec += adjust_cpu_clocks (tv);
zipgrepping in watt32s-v220-d3.zip

[jwt27]

Thanks! Would be nice if that could be retroactively imported to git somehow... But I'm sure that will break something.

I see the correction was previously just:

corr = diff < 0.0 ? -1 : +1;

So then it does do something, and doesn't have a midnight bug. The bug got introduced in v220-d4.

But this algorithm never converges, it always oscillates between two values. Then that introduces jitter, which gets amplified over time as the rdtsc count increases. So to keep it monotonic you'd need some accumulative algorithm (not sure how to describe it). Something like:

static uint64 last_count;
static uint64 last_usec;

uint64 count = get_rdtsc();
uint64 usec = last_usec + (count - last_count) / clocks_per_usec;

last_count = count;
last_usec = usec;

But I think it's best (and easiest) to measure just once and go with that. And it might be worth doing that in floating-point.

[gvanem]

But I think it's best (and easiest) to measure just once and go with that. And it might be worth doing that in floating-point.

OK. I'm not in a position to test this for DOS/djgpp. So just update this PR for float and/or remove adjust_cpu_clock() and I'll merge it real soon.

[jwt27]

OK, will look into it.

[jwt27]

Haven't gotten around to this yet, sorry. Been busy with other stuff. Merging is fine for now, solves the midnight bug at least.