Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Skip task/timer updated if late for more than a few periods #129

Merged
merged 1 commit into from Jan 28, 2016
Merged

Skip task/timer updated if late for more than a few periods #129

merged 1 commit into from Jan 28, 2016

Conversation

pdima
Copy link
Contributor

@pdima pdima commented Jan 13, 2016

I'm not confident the proposed behavior is desired, I'm looking for comments/feedback.

I had the case with system time changed after rtt started, it caused all the periodic activities and timer to
run continuously to catch up all the missing updates since 1970.
I think the similar situation may happen when the system clock changed for other reasons like
daylight or time zone changes.

The better solution would be to use monotonic clocks, but as I understand it can't be used with sem_timedwait.

@pdima
Skip task/timer updated if late for more than a few periods
2318f82 
When the system time is changed ahead due to ntp/time zone/daylight/etc changed,
it's better to skip updates than to run all the activities and timers
continuously to catch up all the missing updates.
@meyerj
Copy link
Member

meyerj commented Jan 28, 2016

Just for the record: There are plans to rewrite the Thread and Timer classes to not depend on real-time clock (system time) anymore, but to use condition variables and monotonic clock sources instead whenever applicable. This would also solve the problem that periodic tasks cannot be stopped before the current waiting period expired.

But as long as we do not have that patch, this PR is a step in the right direction. Nevertheless it should be considered as a workaround. Thanks for the effort!

meyerj added a commit that referenced this pull request Jan 28, 2016
@meyerj
Merge pull request #129 from pdima/skip_late_updates
b683905 
Skip task/timer updated if late for more than a few periods
@meyerj meyerj merged commit b683905 into orocos-toolchain:master Jan 28, 2016
meyerj added a commit that referenced this pull request Aug 10, 2016
@meyerj
Merge pull request #129 from pdima/skip_late_updates
1fc4a17 
Skip task/timer updated if late for more than a few periods
meyerj
meyerj reviewed Oct 23, 2017
View reviewed changes
rtt/os/gnulinux/fosi_internal.cpp
int maxDelayInPeriods = 4;
NANO_TIME period = task->period;
if (now - wake > maxDelayInPeriods*period) {
period = period * ((now - wake) / period);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this should have been

period += period * ((now - wake) / period);

because we always want to add at least one period (line 260) plus eventually the additional ones, like in Timer.cpp:108.

@meyerj
Copy link
Member

meyerj commented Oct 23, 2017

I am thinking about reverting this patch for toolchain-2.9 in favor of #138. #138 only provides a solution for the Thread class, but not for Timer.

Also it should be noted that both patches, #129 and #138, are not effective anymore for periodic components in connection with #91. So actually we would need a clean solution for all these cases of interruptible, perodic waiting by adding an OS abstraction of condition variables which support waiting based on a steady clock source. For gnulinux/pthread that would be using pthread_condattr_setclock with CLOCK_MONOTONIC. And get rid of the semaphore in os::Timer by using a condition variable instead.

@meyerj meyerj mentioned this pull request Oct 23, 2017
Closed
meyerj added a commit to meyerj/rtt that referenced this pull request Mar 1, 2018
@meyerj
Consistently use a monotonic clock source for the gnulinux target
e4f4e83 
The absolute time returned by rtos_get_time_ns() or RTT::os::TimeService::getNSecs() was generated from
a wall clock (time of day), which is not monotonic and can jump in certain cases, e.g. due to an NTP update.
This time is used in several internal and public API functions to implement timers or to wait on a
condition variable, mutex or semaphore with a timeout. In all these cases non-steady system time can have
undesirable side effects, which are not expected in a Real-Time Toolkit.

There have been several attempts to fix or at least mitigate that problem:
- http://bugs.orocos.org/show_bug.cgi?id=735
- orocos-toolchain#129
- orocos-toolchain#138

This patch introduces some potentially breaking changes:
- Use clock_gettime(CLOCK_MONOTONIC, ...) to implement rtos_get_time_ns() for the gnulinux target.
  This change has a major impact on all usages of that method, rtos_get_ticks() or the higher-level
  methods RTT::os::TimeService::getNSecs() and RTT::os::TimeService::getTicks(), whose return values
  cannot be interpreted as wall time anymore.
- Consequently remove the rtos_get_time_monotonic-ns() function introduced in
  orocos-toolchain#138.
- Drop RTT::os::Semaphore::waitUntil() and the underlying OS abstraction functions.
  The only usage within the RTT core was in RTT::os::Timer, which can easily be rewritten using
  condition variables. POSIX semaphores do not support waiting with an absolute timeout from the
  monotonic clock.
- Added new OS abstraction functions for waiting on a mutex lock with a relative timeout. POSIX does
  not provide a pthread_mutexattr_setclock() method similar to pthread_condattr_setclock(). For that
  reason rtos_mutex_lock_until() and rtos_mutex_trylock_for() are currently falling back to
  CLOCK_REALTIME.
meyerj added a commit to meyerj/rtt that referenced this pull request Mar 1, 2018
@meyerj
Consistently use a monotonic clock source for the gnulinux target
1c0ddce 
The absolute time returned by rtos_get_time_ns() or RTT::os::TimeService::getNSecs() was generated from
a wall clock (time of day), which is not monotonic and can jump in certain cases, e.g. due to an NTP update.
This time is used in several internal and public API functions to implement timers or to wait on a
condition variable, mutex or semaphore with a timeout. In all these cases non-steady system time can have
undesirable side effects, which are not expected in a Real-Time Toolkit.

There have been several attempts to fix or at least mitigate that problem:
- http://bugs.orocos.org/show_bug.cgi?id=735
- orocos-toolchain#129
- orocos-toolchain#138

This patch introduces some potentially breaking changes:
- Use clock_gettime(CLOCK_MONOTONIC, ...) to implement rtos_get_time_ns() for the gnulinux target.
  This change has a major impact on all usages of that method, rtos_get_ticks() or the higher-level
  methods RTT::os::TimeService::getNSecs() and RTT::os::TimeService::getTicks(), whose return values
  cannot be interpreted as wall time anymore.
- Consequently remove the rtos_get_time_monotonic-ns() function introduced in
  orocos-toolchain#138.
- Drop RTT::os::Semaphore::waitUntil() and the underlying OS abstraction functions.
  The only usage within the RTT core was in RTT::os::Timer, which can easily be rewritten using
  condition variables. POSIX semaphores do not support waiting with an absolute timeout from the
  monotonic clock.
- Added new OS abstraction functions for waiting on a mutex lock with a relative timeout. POSIX does
  not provide a pthread_mutexattr_setclock() method similar to pthread_condattr_setclock(). For that
  reason rtos_mutex_lock_until() and rtos_mutex_trylock_for() are currently falling back to
  CLOCK_REALTIME.

Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
meyerj added a commit to Intermodalics/rtt that referenced this pull request Mar 1, 2018
@meyerj
Consistently use a monotonic clock source for the gnulinux target
4d0de0e 
The absolute time returned by rtos_get_time_ns() or RTT::os::TimeService::getNSecs() was generated from
a wall clock (time of day), which is not monotonic and can jump in certain cases, e.g. due to an NTP update.
This time source is used in several internal and public API functions to implement timers or to wait on a
condition variable, mutex or semaphore with a timeout. In all these cases non-steady system time can have
undesirable side effects, which are not expected in a Real-Time Toolkit.

There have been several attempts to fix or at least mitigate that problem:
- http://bugs.orocos.org/show_bug.cgi?id=735
- orocos-toolchain#129
- orocos-toolchain#138

This patch introduces some potentially breaking semantic changes, with only minimal changes to the
public API:

- Use clock_gettime(CLOCK_MONOTONIC, ...) to implement rtos_get_time_ns() for the gnulinux target.
  This change has a major impact on all usages of that method, rtos_get_ticks() or the higher-level
  methods RTT::os::TimeService::getNSecs() and RTT::os::TimeService::getTicks(), whose return values
  cannot be interpreted as wall time anymore.
- Consequently remove the rtos_get_time_monotonic_ns() function introduced in
  orocos-toolchain#138.
- Drop RTT::os::Semaphore::waitUntil() and the underlying OS abstraction functions.
  The only usage within the RTT core was in RTT::os::Timer, which can be rewritten using a
  condition variable. POSIX semaphores do not support waiting with an absolute timeout from the
  monotonic clock.
- Add new OS abstraction functions for waiting on a mutex lock with a relative timeout and use them
  to implement RTT::os::Mutex::timedlock(). POSIX does not provide a pthread_mutexattr_setclock()
  method similar to pthread_condattr_setclock(). For that reason rtos_mutex_lock_until() and
  rtos_mutex_trylock_for() have to fall back to CLOCK_REALTIME and may still be affected by time
  adjustments while waiting.

Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
meyerj added a commit to Intermodalics/rtt that referenced this pull request Mar 1, 2018
@meyerj
Use a monotonic clock source whenever possible (for the gnulinux target)
67e1a10 
The absolute time returned by rtos_get_time_ns() or RTT::os::TimeService::getNSecs() was generated from
a wall clock (time of day), which is not monotonic and can jump in certain cases, e.g. due to an NTP update.
This time source is used in several internal and public API functions to implement timers or to wait on a
condition variable, mutex or semaphore with a timeout. In all these cases non-steady system time can have
undesirable side effects, which are not expected in a Real-Time Toolkit.

There have been several attempts to fix or at least mitigate that problem:
- http://bugs.orocos.org/show_bug.cgi?id=735
- orocos-toolchain#129
- orocos-toolchain#138

This patch introduces some potentially breaking semantic changes, with only minimal changes to the
public API:

- Use clock_gettime(CLOCK_MONOTONIC, ...) to implement rtos_get_time_ns() for the gnulinux target.
  This change has a major impact on all usages of that method, rtos_get_ticks() or the higher-level
  methods RTT::os::TimeService::getNSecs() and RTT::os::TimeService::getTicks(), whose return values
  cannot be interpreted as wall time anymore.
- Consequently remove the rtos_get_time_monotonic_ns() function introduced in
  orocos-toolchain#138.
- Drop RTT::os::Semaphore::waitUntil() and the underlying OS abstraction functions.
  The only usage within the RTT core was in RTT::os::Timer, which can be rewritten using a
  condition variable. POSIX semaphores do not support waiting with an absolute timeout from the
  monotonic clock.
- Add new OS abstraction functions for waiting on a mutex lock with a relative timeout and use them
  to implement RTT::os::Mutex::timedlock(). POSIX does not provide a pthread_mutexattr_setclock()
  method similar to pthread_condattr_setclock(). For that reason rtos_mutex_lock_until() and
  rtos_mutex_trylock_for() have to fall back to CLOCK_REALTIME and may still be affected by time
  adjustments while waiting.

Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
@meyerj meyerj mentioned this pull request Mar 1, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meyerj meyerj meyerj left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pdima @meyerj