buserror / simavr

simavr is a lean, mean and hackable AVR simulator for linux & OSX
GNU General Public License v3.0
1.58k stars 368 forks source link

Wall clock time and simulation time diverge #254

Closed dxxb closed 6 years ago

dxxb commented 7 years ago

This is somewhat related to #9. I believe the intent of the current implementation of avr_callback_sleep_raw() is to run the simulation close to 1x speed.

void
avr_callback_sleep_raw(
        avr_t * avr,
        avr_cycle_count_t howLong)
{
    uint32_t usec = avr_pending_sleep_usec(avr, howLong);
    if (usec > 0) {
        usleep(usec);
    }
}

However, the simulation of a cycle is not instantaneous (obviously) and usleep() duration errors accumulate over time so simulation time and wall clock time diverge (i.e. simulation_start_time+avr->cycle*nominal_cycle_duration << simulation_start_time+elapsed_wall_clock_time).

In order to keep the simulation time close to 1x (or Nx if the host CPU is fast enough) the elapsed wall clock time has to be taken into account. I have successfully used the following replacement for avr_callback_sleep_raw().

void avr_callback_sleep_sync_1x(
        avr_t * avr,
        avr_cycle_count_t howLong)
{
    struct timespec tp;

    /* figure out how log we should wait to match the sleep deadline */
    uint64_t deadline_ns = avr_cycles_to_nsec(avr, avr->cycle + howLong);
    clock_gettime(CLOCK_MONOTONIC_RAW, &tp);
    uint64_t runtime_ns = (tp.tv_sec*1000000000+tp.tv_nsec) - start_time_ns;
    if (runtime_ns >= deadline_ns) {
        return;
    }

    uint64_t sleep_us = (deadline_ns - runtime_ns)/1000;
    usleep(sleep_us);   
    return;
}

The function above can be easily modified to support running the simulation at an arbitrary programmable rate r as long as the host is fast enough to keep up.

buserror commented 7 years ago

To be fair I never really tried terribly hard to get the simulation to x1 speed -- as technically that would requires slowing down the cycles themselves. Instead what we do is 'burst' thru things, and in only works if the firmware is 'sleep friendly'...

Mind you I'm not unhappy about that either, however, before I merge we need to fix it for OSX as It'd guaranteed to break on there...

dxxb commented 7 years ago

To be fair I never really tried terribly hard to get the simulation to x1 speed -- as technically that would requires slowing down the cycles themselves. Instead what we do is 'burst' thru things

Yes, and the solution above just sets up a feedback loop between CLOCK_MONOTONIC_RAW and simulation time. It will not be exactly 1x but will rein it back in if it goes too far between bursts.

before I merge we need to fix it for OSX as It'd guaranteed to break on there...

FYI, I am testing on OSX and it works there.

buserror commented 7 years ago

So what about rolling this in in a PR @dxxb ?

dxxb commented 7 years ago

Hi @buserror, yes absolutely! I didn't realise you were ok with this. Are you ok with substituting the implementation of avr_callback_sleep_raw()?

buserror commented 7 years ago

Go for it, looks like a better implementation for this particular feature :-)

dxxb commented 7 years ago

ack, I have it on a branch already. Will push and open a PR later today. However, I found out clock_gettime() is supported only in OSX >= 10.12 . My commit as-is would make simavr build only on OSX 10.12 and later. Not sure this is acceptable.

buserror commented 7 years ago

Hmm merged before I saw this last comment. I'll try to compile it on an old OSX later. Theorically you could do the same feature with gettimeofday() -- without requiring the MONOTONIC bit. I used gettimeofday before for doing that sort of drift correction before and it works fine.

dxxb commented 7 years ago

I used a monotonic clock because it is not affected by system time adjustments (i.e. it won't jump forwards or backwards while the simulation is running).

On the other hand upgrading OSX is free of charge. FYI my two Macs from 2010 can be upgraded to the latest OSX 10.13.

bsekisser commented 6 years ago

putting at the top may partially solve the problem (ie visible on systems that have it, may need to define -lrt):...

define _POSIX_C_SOURCE 199309L

HOWEVER!!! This creates a bit of another problem... use of usleep is declared obsolete and may be unavailable with above defined (as in my case). in which case... use of nanosleep is called for.

I think I seen another macro you can check for to see if above is available, but, don't know how to use it as of yet... nor can I find it at the moment.

my system has clock_gettime, but not active... debian 7.11.

buserror commented 6 years ago

I pushed a more portable fix for this. Using 'obsolete' gettimeofday(), that'll probably still be around for my lifetime anyway ;-)