Skip to content
Open

Usleep #1597

Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 87 additions & 2 deletions common/utils/Clock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
#include <sstream>
#include <string>

#include "ola/Logging.h"

namespace ola {

using std::string;
Expand Down Expand Up @@ -125,8 +127,8 @@ void BaseTimeVal::AsTimeval(struct timeval *tv) const {
}

int64_t BaseTimeVal::InMilliSeconds() const {
return (m_tv.tv_sec * static_cast<int64_t>(ONE_THOUSAND) +
m_tv.tv_usec / ONE_THOUSAND);
return (m_tv.tv_sec * static_cast<int64_t>(MSEC_IN_SEC) +
m_tv.tv_usec / MSEC_IN_SEC);
}

int64_t BaseTimeVal::AsInt() const {
Expand Down Expand Up @@ -272,4 +274,87 @@ void MockClock::CurrentTime(TimeStamp *timestamp) const {
*timestamp = tv;
*timestamp += m_offset;
}

Sleep::Sleep(std::string caller) :
m_caller(caller) {
}

/**
* @brief Set wanted granularity for usleep and check it.
* @note does not check at the nanosecond level,
* since internal sturtures use usecs.
*
* @param wanted wanted/needed granularity in usecs
* @param maxDeviation max deviation in usecs tolerated by calling thread.
*
* @attention the granularity of sleep is highly fluctuating depending on the
* load of the system, a prior GOOD state is no guarantee for future proper
* timing.
*/
bool Sleep::CheckTimeGranularity(uint64_t wanted, uint64_t maxDeviation) {
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be called Check... given it's actually usleeping. Should there not also be one that uses the defined wanting and max deviation times? Test maybe, or ideally a similar synonym.

Copy link
Author

Choose a reason for hiding this comment

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

It's usleeping but it's only meant to be used to get a rough idea of how accurate usleep is after which further sleeps should be done with Usleep()

(which still remains a total crapshoot no matter what unless we/I find out if there is a way to trigger the program to be run in a real time fashion but afaik that is changing the kernel scheduler settings, so that would just be adding docs that "if the user wants really acurate timings they should do ....")

TimeStamp ts1, ts2;
Clock clock;

m_wanted_granularity = wanted;
m_max_granularity_deviation = maxDeviation;

timespec t;
t.tv_sec = wanted / USEC_IN_SECONDS;
t.tv_nsec = (wanted % USEC_IN_SECONDS) * ONE_THOUSAND;

clock.CurrentTime(&ts1);
this->usleep(1);
clock.CurrentTime(&ts2);
TimeInterval interval = ts2 - ts1;
m_clock_overhead = interval.InMicroSeconds();

clock.CurrentTime(&ts1);
this->usleep(t);
clock.CurrentTime(&ts2);

interval = ts2 - ts1;
m_granularity = (interval.InMicroSeconds() >
(wanted + maxDeviation + m_clock_overhead)) ? BAD : GOOD;

OLA_INFO << "Granularity for OlaSleep in " << m_caller << " is "
<< ((m_granularity == GOOD) ? "GOOD" : "BAD")
<< " Requested: " << wanted << " Got: " << interval.InMicroSeconds()
<< " Overhead: " << m_clock_overhead;
if (m_granularity == GOOD) {
return true;
}
return false;
}

void Sleep::usleep(TimeInterval requested) {
timespec req;
req.tv_sec = requested.Seconds();
req.tv_nsec = requested.MicroSeconds() * ONE_THOUSAND;

this->usleep(req);
}

void Sleep::usleep(uint32_t requested) {
timespec req;
Copy link
Member

Choose a reason for hiding this comment

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

This should be using the existing maths in TimeInterval rather than rewriting that code here.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure I follow, we have a TimeIntervalToTimeSpec function somewhere?

Copy link
Author

Choose a reason for hiding this comment

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

(It does make sense to add such a function but I did not see any such thing listed in the header)

req.tv_sec = requested / USEC_IN_SECONDS;
req.tv_nsec = (requested % USEC_IN_SECONDS) * ONE_THOUSAND;

this->usleep(req);
}

void Sleep::usleep(timespec requested) {
timespec rem;

if (nanosleep(&requested, &rem) < 0) {
if (errno == EINTR) {
while (rem.tv_nsec > 0 || rem.tv_sec > 0) {
requested.tv_nsec = rem.tv_nsec;
requested.tv_sec = rem.tv_sec;
nanosleep(&requested, &rem);
}
} else {
OLA_WARN << "nanosleep failed with state: " << errno;
}
}
}
} // namespace ola
39 changes: 38 additions & 1 deletion include/ola/Clock.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@

namespace ola {

static const uint64_t NSEC_IN_SEC = 1000000000;
static const int USEC_IN_SECONDS = 1000000;
static const int ONE_THOUSAND = 1000;
static const int MSEC_IN_SEC = 1000;
static const int ONE_THOUSAND = MSEC_IN_SEC;

/**
* Don't use this class directly. It's an implementation detail of TimeInterval
Expand Down Expand Up @@ -104,6 +106,12 @@ class BaseTimeVal {
*/
int64_t AsInt() const;

/**
* @brief InMicroSeconds wrapper for AsInt() for name consistency.
* @return The entire BaseTimeVal in microseconds
*/
int64_t InMicroSeconds() const { return this->AsInt(); }

std::string ToString() const;

private:
Expand Down Expand Up @@ -160,6 +168,7 @@ class TimeInterval {
int32_t MicroSeconds() const { return m_interval.MicroSeconds(); }

int64_t InMilliSeconds() const { return m_interval.InMilliSeconds(); }
int64_t InMicroSeconds() const { return m_interval.InMicroSeconds(); }
Copy link
Member

Choose a reason for hiding this comment

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

Please can you correct the CamelCase of this and it's siblings to InMicroseconds (and throughout the codebase). We'll just add a note to the NEWS file like when we renamed DMXRecieved(sic) to DMXReceived.

Copy link
Author

Choose a reason for hiding this comment

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

Did this, should I add a note to NEWS or will you once you merge?

int64_t AsInt() const { return m_interval.AsInt(); }

std::string ToString() const { return m_interval.ToString(); }
Expand Down Expand Up @@ -257,5 +266,33 @@ class MockClock: public Clock {
private:
TimeInterval m_offset;
};

enum TimerGranularity { UNKNOWN, GOOD, BAD };

/**
* @brief The Sleep class implements usleep with some granualtiry detection.
*/
class Sleep {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should probably be called GranularSleep or something should we ever have to do something like wrap/override usleep or similar.

Copy link
Author

Choose a reason for hiding this comment

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

The whole point of this class is to do usleep without the various test systems screaming at us that usleep shouldn't be used.

public:
explicit Sleep(std::string caller);

void setCaller(std::string caller) { m_caller = caller; }

void usleep(TimeInterval requested);
void usleep(uint32_t requested);
void usleep(timespec requested);

TimerGranularity getGranularity() { return m_granularity; }
bool CheckTimeGranularity(uint64_t wanted, uint64_t maxDeviation);
private:
std::string m_caller;
uint64_t m_wanted_granularity;
uint64_t m_max_granularity_deviation;
uint64_t m_clock_overhead;

static const uint32_t BAD_GRANULARITY_LIMIT = 10;

TimerGranularity m_granularity = UNKNOWN;
};
} // namespace ola
#endif // INCLUDE_OLA_CLOCK_H_