Skip to content

Commit

Permalink
Request the realtime and monotonic clock times once per sample
Browse files Browse the repository at this point in the history 
Refactor the sample time code to make one call to gettimeofday
(aka the realtime clock in clock_gettime, when available) and
one to the monotonic clock.  Stores each in more appropriately
named ProcessList fields for ready access when needed.  Every
platform gets the opportunity to provide their own clock code,
and the existing Mac OS X specific code is moved below darwin
instead of in Compat.

A couple of leftover time(2) calls are converted to use these
ProcessList fields as well, instead of yet again sampling the
system clock.

Related to #574
  • Loading branch information
@natoscott @smalinux
natoscott authored and smalinux committed Apr 5, 2021
1 parent 421bdee commit 356488a
Show file tree
Hide file tree
Showing 25 changed files with 210 additions and 82 deletions.
2 changes: 1 addition & 1 deletion ClockMeter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ static void ClockMeter_updateValues(Meter* this) {
const ProcessList* pl = this->pl;

struct tm result;
const struct tm* lt = localtime_r(&pl->timestamp.tv_sec, &result);
const struct tm* lt = localtime_r(&pl->realtime.tv_sec, &result);
this->values[0] = lt->tm_hour * 60 + lt->tm_min;
strftime(this->txtBuffer, sizeof(this->txtBuffer), "%H:%M:%S", lt);
}
Expand Down
34 changes: 0 additions & 34 deletions Compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,12 @@ in the source distribution for its full text.

#include <errno.h>
#include <fcntl.h> // IWYU pragma: keep
#include <time.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h> // IWYU pragma: keep

#include "XUtils.h" // IWYU pragma: keep

#ifdef HAVE_HOST_GET_CLOCK_SERVICE
#include <mach/clock.h>
#include <mach/mach.h>
#endif


int Compat_faccessat(int dirfd,
const char* pathname,
Expand Down Expand Up @@ -123,31 +117,3 @@ ssize_t Compat_readlinkat(int dirfd,

#endif
}

int Compat_clock_monotonic_gettime(struct timespec *tp) {

#if defined(HAVE_CLOCK_GETTIME)

return clock_gettime(CLOCK_MONOTONIC, tp);

#elif defined(HAVE_HOST_GET_CLOCK_SERVICE)

clock_serv_t cclock;
mach_timespec_t mts;

host_get_clock_service(mach_host_self(), SYSTEM_CLOCK, &cclock);
clock_get_time(cclock, &mts);
mach_port_deallocate(mach_task_self(), cclock);

tp->tv_sec = mts.tv_sec;
tp->tv_nsec = mts.tv_nsec;

return 0;

#else

#error No Compat_clock_monotonic_gettime() implementation!

#endif

}
2 changes: 0 additions & 2 deletions Compat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,4 @@ ssize_t Compat_readlinkat(int dirfd,
char* buf,
size_t bufsize);

int Compat_clock_monotonic_gettime(struct timespec *tp);

#endif /* HEADER_Compat */
2 changes: 1 addition & 1 deletion DateMeter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ static void DateMeter_updateValues(Meter* this) {
const ProcessList* pl = this->pl;

struct tm result;
const struct tm* lt = localtime_r(&pl->timestamp.tv_sec, &result);
const struct tm* lt = localtime_r(&pl->realtime.tv_sec, &result);
this->values[0] = lt->tm_yday;
int year = lt->tm_year + 1900;
if (((year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0)) {
Expand Down
5 changes: 3 additions & 2 deletions DateTimeMeter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ static const int DateTimeMeter_attributes[] = {
};

static void DateTimeMeter_updateValues(Meter* this) {
time_t t = time(NULL);
const ProcessList* pl = this->pl;

struct tm result;
const struct tm* lt = localtime_r(&t, &result);
const struct tm* lt = localtime_r(&pl->realtime.tv_sec, &result);
int year = lt->tm_year + 1900;
if (((year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0)) {
this->total = 366;
Expand Down
4 changes: 2 additions & 2 deletions DiskIOMeter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ static void DiskIOMeter_updateValues(Meter* this) {
const ProcessList* pl = this->pl;

static uint64_t cached_last_update;
uint64_t passedTimeInMs = pl->timestampMs - cached_last_update;
uint64_t passedTimeInMs = pl->realtimeMs - cached_last_update;

/* update only every 500ms */
if (passedTimeInMs > 500) {
Expand All @@ -43,7 +43,7 @@ static void DiskIOMeter_updateValues(Meter* this) {
static uint64_t cached_msTimeSpend_total;
uint64_t diff;

cached_last_update = pl->timestampMs;
cached_last_update = pl->realtimeMs;

DiskIOData data;

Expand Down
14 changes: 14 additions & 0 deletions Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ myhtopheaders = \
# -----

linux_platform_headers = \
generic/gettime.h \
generic/hostname.h \
generic/uname.h \
linux/HugePageMeter.h \
Expand All @@ -154,6 +155,7 @@ linux_platform_headers = \
zfs/ZfsCompressedArcMeter.h

linux_platform_sources = \
generic/gettime.c \
generic/hostname.c \
generic/uname.c \
linux/HugePageMeter.c \
Expand Down Expand Up @@ -184,6 +186,7 @@ freebsd_platform_headers = \
freebsd/FreeBSDProcess.h \
freebsd/Platform.h \
freebsd/ProcessField.h \
generic/gettime.h \
generic/hostname.h \
generic/openzfs_sysctl.h \
generic/uname.h \
Expand All @@ -195,6 +198,7 @@ freebsd_platform_sources = \
freebsd/Platform.c \
freebsd/FreeBSDProcessList.c \
freebsd/FreeBSDProcess.c \
generic/gettime.c \
generic/hostname.c \
generic/openzfs_sysctl.c \
generic/uname.c \
Expand All @@ -215,13 +219,15 @@ dragonflybsd_platform_headers = \
dragonflybsd/DragonFlyBSDProcess.h \
dragonflybsd/Platform.h \
dragonflybsd/ProcessField.h \
generic/gettime.h \
generic/hostname.h \
generic/uname.h

dragonflybsd_platform_sources = \
dragonflybsd/DragonFlyBSDProcessList.c \
dragonflybsd/DragonFlyBSDProcess.c \
dragonflybsd/Platform.c \
generic/gettime.c \
generic/hostname.c \
generic/uname.c

Expand All @@ -235,6 +241,7 @@ endif
# -------

openbsd_platform_headers = \
generic/gettime.h \
generic/hostname.h \
generic/uname.h \
openbsd/OpenBSDProcessList.h \
Expand All @@ -243,6 +250,7 @@ openbsd_platform_headers = \
openbsd/ProcessField.h

openbsd_platform_sources = \
generic/gettime.c \
generic/hostname.c \
generic/uname.c \
openbsd/OpenBSDProcessList.c \
Expand All @@ -263,6 +271,7 @@ darwin_platform_headers = \
darwin/DarwinProcessList.h \
darwin/Platform.h \
darwin/ProcessField.h \
generic/gettime.h \
generic/hostname.h \
generic/openzfs_sysctl.h \
generic/uname.h \
Expand All @@ -274,6 +283,7 @@ darwin_platform_sources = \
darwin/Platform.c \
darwin/DarwinProcess.c \
darwin/DarwinProcessList.c \
generic/gettime.c \
generic/hostname.c \
generic/openzfs_sysctl.c \
generic/uname.c \
Expand All @@ -291,6 +301,7 @@ endif
# -------

solaris_platform_headers = \
generic/gettime.h \
generic/hostname.h \
generic/uname.h \
solaris/ProcessField.h \
Expand All @@ -302,6 +313,7 @@ solaris_platform_headers = \
zfs/ZfsCompressedArcMeter.h

solaris_platform_sources = \
generic/gettime.c \
generic/hostname.c \
generic/uname.c \
solaris/Platform.c \
Expand All @@ -320,12 +332,14 @@ endif
# -----------

unsupported_platform_headers = \
generic/gettime.h \
unsupported/Platform.h \
unsupported/ProcessField.h \
unsupported/UnsupportedProcess.h \
unsupported/UnsupportedProcessList.h

unsupported_platform_sources = \
generic/gettime.c \
unsupported/Platform.c \
unsupported/UnsupportedProcess.c \
unsupported/UnsupportedProcessList.c
Expand Down
4 changes: 2 additions & 2 deletions Meter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,10 @@ static void GraphMeterMode_draw(Meter* this, int x, int y, int w) {
x += captionLen;
w -= captionLen;

if (!timercmp(&pl->timestamp, &(data->time), <)) {
if (!timercmp(&pl->realtime, &(data->time), <)) {
int globalDelay = this->pl->settings->delay;
struct timeval delay = { .tv_sec = globalDelay / 10, .tv_usec = (globalDelay - ((globalDelay / 10) * 10)) * 100000 };
timeradd(&pl->timestamp, &delay, &(data->time));
timeradd(&pl->realtime, &delay, &(data->time));

for (int i = 0; i < nValues - 1; i++)
data->values[i] = data->values[i + 1];
Expand Down
4 changes: 2 additions & 2 deletions NetworkIOMeter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static void NetworkIOMeter_updateValues(Meter* this) {
const ProcessList* pl = this->pl;
static uint64_t cached_last_update = 0;

uint64_t passedTimeInMs = pl->timestampMs - cached_last_update;
uint64_t passedTimeInMs = pl->realtimeMs - cached_last_update;

/* update only every 500ms */
if (passedTimeInMs > 500) {
Expand All @@ -38,7 +38,7 @@ static void NetworkIOMeter_updateValues(Meter* this) {
static uint64_t cached_txp_total;
uint64_t diff;

cached_last_update = pl->timestampMs;
cached_last_update = pl->realtimeMs;

NetworkIOData data;
hasData = Platform_getNetworkIO(&data);
Expand Down
6 changes: 3 additions & 3 deletions Process.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,14 @@ void Process_toggleTag(Process* this) {

bool Process_isNew(const Process* this) {
assert(this->processList);
if (this->processList->scanTs >= this->seenTs) {
return this->processList->scanTs - this->seenTs <= 1000 * this->processList->settings->highlightDelaySecs;
if (this->processList->monotonicMs >= this->seenStampMs) {
return this->processList->monotonicMs - this->seenStampMs <= 1000 * (uint64_t)this->processList->settings->highlightDelaySecs;
}
return false;
}

bool Process_isTomb(const Process* this) {
return this->tombTs > 0;
return this->tombStampMs > 0;
}

bool Process_setPriority(Process* this, int priority) {
Expand Down
4 changes: 2 additions & 2 deletions Process.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ typedef struct Process_ {
/*
* Internal time counts for showing new and exited processes.
*/
time_t seenTs;
time_t tombTs;
uint64_t seenStampMs;
uint64_t tombStampMs;

/*
* Internal state for tree-mode.
Expand Down
27 changes: 12 additions & 15 deletions ProcessList.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ in the source distribution for its full text.
#include <assert.h>
#include <stdlib.h>
#include <string.h>
#include <sys/time.h>

#include "Compat.h"
#include "CRT.h"
#include "Hashtable.h"
#include "Macros.h"
#include "Platform.h"
#include "Vector.h"
#include "XUtils.h"

Expand All @@ -35,7 +36,7 @@ ProcessList* ProcessList_init(ProcessList* this, const ObjectClass* klass, Users
// set later by platform-specific code
this->cpuCount = 0;

this->scanTs = 0;
this->monotonicMs = 0;

#ifdef HAVE_LIBHWLOC
this->topologyOk = false;
Expand Down Expand Up @@ -131,7 +132,7 @@ void ProcessList_add(ProcessList* this, Process* p) {
p->processList = this;

// highlighting processes found in first scan by first scan marked "far in the past"
p->seenTs = this->scanTs;
p->seenStampMs = this->monotonicMs;

Vector_add(this->processes, p);
Hashtable_put(this->processTable, p->pid, p);
Expand Down Expand Up @@ -582,8 +583,6 @@ Process* ProcessList_getProcess(ProcessList* this, pid_t pid, bool* preExisting,
}

void ProcessList_scan(ProcessList* this, bool pauseProcessUpdate) {
struct timespec now;

// in pause mode only gather global data for meters (CPU/memory/...)
if (pauseProcessUpdate) {
ProcessList_goThroughEntries(this, true);
Expand All @@ -604,31 +603,29 @@ void ProcessList_scan(ProcessList* this, bool pauseProcessUpdate) {
this->runningTasks = 0;


// set scanTs
// set scan timestamp
static bool firstScanDone = false;
if (!firstScanDone) {
this->scanTs = 0;
if (firstScanDone) {
Platform_gettime_monotonic(&this->monotonicMs);
} else {
this->monotonicMs = 0;
firstScanDone = true;
} else if (Compat_clock_monotonic_gettime(&now) == 0) {
// save time in millisecond, so with a delay in deciseconds
// there are no irregularities
this->scanTs = 1000 * now.tv_sec + now.tv_nsec / 1000000;
}

ProcessList_goThroughEntries(this, false);

for (int i = Vector_size(this->processes) - 1; i >= 0; i--) {
Process* p = (Process*) Vector_get(this->processes, i);
if (p->tombTs > 0) {
if (p->tombStampMs > 0) {
// remove tombed process
if (this->scanTs >= p->tombTs) {
if (this->monotonicMs >= p->tombStampMs) {
ProcessList_remove(this, p);
}
} else if (p->updated == false) {
// process no longer exists
if (this->settings->highlightChanges && p->wasShown) {
// mark tombed
p->tombTs = this->scanTs + 1000 * this->settings->highlightDelaySecs;
p->tombStampMs = this->monotonicMs + 1000 * this->settings->highlightDelaySecs;
} else {
// immediately remove
ProcessList_remove(this, p);
Expand Down
7 changes: 3 additions & 4 deletions ProcessList.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ typedef struct ProcessList_ {
Hashtable* displayTreeSet;
Hashtable* draftingTreeSet;

struct timeval timestamp; /* time of the current sample */
uint64_t timestampMs; /* current time in milliseconds */
struct timeval realtime; /* time of the current sample */
uint64_t realtimeMs; /* current time in milliseconds */
uint64_t monotonicMs; /* same, but from monotonic clock */

Panel* panel;
int following;
Expand Down Expand Up @@ -79,8 +80,6 @@ typedef struct ProcessList_ {
memory_t cachedSwap;

unsigned int cpuCount;

time_t scanTs;
} ProcessList;

ProcessList* ProcessList_new(UsersTable* usersTable, Hashtable* pidMatchList, uid_t userId);
Expand Down
6 changes: 3 additions & 3 deletions ScreenManager.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ in the source distribution for its full text.
#include "CRT.h"
#include "FunctionBar.h"
#include "Object.h"
#include "Platform.h"
#include "ProcessList.h"
#include "ProvideCurses.h"
#include "XUtils.h"
Expand Down Expand Up @@ -92,9 +93,8 @@ void ScreenManager_resize(ScreenManager* this, int x1, int y1, int x2, int y2) {
static void checkRecalculation(ScreenManager* this, double* oldTime, int* sortTimeout, bool* redraw, bool* rescan, bool* timedOut) {
ProcessList* pl = this->header->pl;

struct timeval tv;
gettimeofday(&tv, NULL);
double newTime = ((double)tv.tv_sec * 10) + ((double)tv.tv_usec / 100000);
Platform_gettime_realtime(&pl->realtime, &pl->realtimeMs);
double newTime = ((double)pl->realtime.tv_sec * 10) + ((double)pl->realtime.tv_usec / 100000);

*timedOut = (newTime - *oldTime > this->settings->delay);
*rescan |= *timedOut;
Expand Down
Loading

1 comment on commit 356488a

@elder-n00b
Copy link

@elder-n00b elder-n00b commented on 356488a Sep 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For information, this breaks on macos 10.11 where monotonic clock is not available.
I think a workaround is possible, I'll look into that. edit: The workaround is the existing code in Platform_gettime_monotonic in darwin/Platform.c but Generic_gettime_monotonic in gettime.c errors "No monotonic clock available". Removing the #error ... seems to almost work, but the times do not always look right to me -- many are zero that the 3.0.5 version shows as something above zero, indeed they're all rounded to minutes.
BTW, there's a "monotomic" typo in darwin/Platform.c line 428. It's still around in HEAD.

Please sign in to comment.