From 421bdeec603b4fb1a4edec0e802c437fbe47fca0 Mon Sep 17 00:00:00 2001 From: Sohaib Date: Tue, 23 Mar 2021 08:27:05 +0200 Subject: [PATCH] Merging all the points related to calculating time in one place The end goal is to consolidate all the points in htop that can only work in live-only mode today, so that will be able to inject PCP archive mode and have a chance at it working. The biggest problem we've got at this moment is all the places that are independently asking the kernel to 'give me the time right now'. Each of those needs to be audited and ultimately changed to allow platforms to manage their own idea of time. So, all the calls to gettimeofday(2) and time(2) are potential problems. Ultimately I want to get these down to just one or two. Related to https://github.com/htop-dev/htop/pull/574 --- ClockMeter.c | 5 +++-- DateMeter.c | 5 +++-- DiskIOMeter.c | 10 ++++------ Meter.c | 8 ++++---- NetworkIOMeter.c | 8 +++----- ProcessList.h | 3 +++ linux/LinuxProcessList.c | 14 +++++++++----- 7 files changed, 29 insertions(+), 24 deletions(-) diff --git a/ClockMeter.c b/ClockMeter.c index 410c7e8a5..de47bdcbe 100644 --- a/ClockMeter.c +++ b/ClockMeter.c @@ -20,9 +20,10 @@ static const int ClockMeter_attributes[] = { }; static void ClockMeter_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->timestamp.tv_sec, &result); this->values[0] = lt->tm_hour * 60 + lt->tm_min; strftime(this->txtBuffer, sizeof(this->txtBuffer), "%H:%M:%S", lt); } diff --git a/DateMeter.c b/DateMeter.c index ac653b624..e7fe1a672 100644 --- a/DateMeter.c +++ b/DateMeter.c @@ -20,9 +20,10 @@ static const int DateMeter_attributes[] = { }; static void DateMeter_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->timestamp.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)) { diff --git a/DiskIOMeter.c b/DiskIOMeter.c index dde007ed1..c1b356903 100644 --- a/DiskIOMeter.c +++ b/DiskIOMeter.c @@ -31,12 +31,10 @@ static uint32_t cached_write_diff; static double cached_utilisation_diff; static void DiskIOMeter_updateValues(Meter* this) { - static uint64_t cached_last_update; + const ProcessList* pl = this->pl; - struct timeval tv; - gettimeofday(&tv, NULL); - uint64_t timeInMilliSeconds = (uint64_t)tv.tv_sec * 1000 + (uint64_t)tv.tv_usec / 1000; - uint64_t passedTimeInMs = timeInMilliSeconds - cached_last_update; + static uint64_t cached_last_update; + uint64_t passedTimeInMs = pl->timestampMs - cached_last_update; /* update only every 500ms */ if (passedTimeInMs > 500) { @@ -45,7 +43,7 @@ static void DiskIOMeter_updateValues(Meter* this) { static uint64_t cached_msTimeSpend_total; uint64_t diff; - cached_last_update = timeInMilliSeconds; + cached_last_update = pl->timestampMs; DiskIOData data; diff --git a/Meter.c b/Meter.c index 6aa9f08e0..b60b69334 100644 --- a/Meter.c +++ b/Meter.c @@ -18,6 +18,7 @@ in the source distribution for its full text. #include "CRT.h" #include "Macros.h" #include "Object.h" +#include "Platform.h" #include "ProvideCurses.h" #include "RichString.h" #include "Settings.h" @@ -286,6 +287,7 @@ static const char* const GraphMeterMode_dotsAscii[] = { }; static void GraphMeterMode_draw(Meter* this, int x, int y, int w) { + const ProcessList* pl = this->pl; if (!this->drawData) { this->drawData = xCalloc(1, sizeof(GraphData)); @@ -312,12 +314,10 @@ static void GraphMeterMode_draw(Meter* this, int x, int y, int w) { x += captionLen; w -= captionLen; - struct timeval now; - gettimeofday(&now, NULL); - if (!timercmp(&now, &(data->time), <)) { + if (!timercmp(&pl->timestamp, &(data->time), <)) { int globalDelay = this->pl->settings->delay; struct timeval delay = { .tv_sec = globalDelay / 10, .tv_usec = (globalDelay - ((globalDelay / 10) * 10)) * 100000 }; - timeradd(&now, &delay, &(data->time)); + timeradd(&pl->timestamp, &delay, &(data->time)); for (int i = 0; i < nValues - 1; i++) data->values[i] = data->values[i + 1]; diff --git a/NetworkIOMeter.c b/NetworkIOMeter.c index c0e1b06fc..bfcb3c0ee 100644 --- a/NetworkIOMeter.c +++ b/NetworkIOMeter.c @@ -25,12 +25,10 @@ static uint32_t cached_txb_diff; static uint32_t cached_txp_diff; static void NetworkIOMeter_updateValues(Meter* this) { + const ProcessList* pl = this->pl; static uint64_t cached_last_update = 0; - struct timeval tv; - gettimeofday(&tv, NULL); - uint64_t timeInMilliSeconds = (uint64_t)tv.tv_sec * 1000 + (uint64_t)tv.tv_usec / 1000; - uint64_t passedTimeInMs = timeInMilliSeconds - cached_last_update; + uint64_t passedTimeInMs = pl->timestampMs - cached_last_update; /* update only every 500ms */ if (passedTimeInMs > 500) { @@ -40,7 +38,7 @@ static void NetworkIOMeter_updateValues(Meter* this) { static uint64_t cached_txp_total; uint64_t diff; - cached_last_update = timeInMilliSeconds; + cached_last_update = pl->timestampMs; NetworkIOData data; hasData = Platform_getNetworkIO(&data); diff --git a/ProcessList.h b/ProcessList.h index 3d9b8ff2c..46944dbd7 100644 --- a/ProcessList.h +++ b/ProcessList.h @@ -48,6 +48,9 @@ typedef struct ProcessList_ { Hashtable* displayTreeSet; Hashtable* draftingTreeSet; + struct timeval timestamp; /* time of the current sample */ + uint64_t timestampMs; /* current time in milliseconds */ + Panel* panel; int following; uid_t userId; diff --git a/linux/LinuxProcessList.c b/linux/LinuxProcessList.c index b12a415b0..c6fd205af 100644 --- a/linux/LinuxProcessList.c +++ b/linux/LinuxProcessList.c @@ -189,6 +189,13 @@ static void LinuxProcessList_updateCPUcount(ProcessList* super, FILE* stream) { } } +static void LinuxProcessList_updateTime(LinuxProcessList* this) { + ProcessList* pl = &(this->super); + + gettimeofday(&pl->timestamp, NULL); + pl->timestampMs = (uint64_t)&pl->timestamp.tv_sec * 1000 + (uint64_t)pl->timestamp.tv_usec / 1000; +} + ProcessList* ProcessList_new(UsersTable* usersTable, Hashtable* pidMatchList, uid_t userId) { LinuxProcessList* this = xCalloc(1, sizeof(LinuxProcessList)); ProcessList* pl = &(this->super); @@ -1968,6 +1975,7 @@ void ProcessList_goThroughEntries(ProcessList* super, bool pauseProcessUpdate) { LinuxProcessList* this = (LinuxProcessList*) super; const Settings* settings = super->settings; + LinuxProcessList_updateTime(this); LinuxProcessList_scanMemoryInfo(super); LinuxProcessList_scanHugePages(this); LinuxProcessList_scanZfsArcstats(this); @@ -1989,10 +1997,6 @@ void ProcessList_goThroughEntries(ProcessList* super, bool pauseProcessUpdate) { return; } - struct timeval tv; - gettimeofday(&tv, NULL); - unsigned long long now = tv.tv_sec * 1000ULL + tv.tv_usec / 1000ULL; - /* PROCDIR is an absolute path */ assert(PROCDIR[0] == '/'); #ifdef HAVE_OPENAT @@ -2001,5 +2005,5 @@ void ProcessList_goThroughEntries(ProcessList* super, bool pauseProcessUpdate) { openat_arg_t rootFd = ""; #endif - LinuxProcessList_recurseProcTree(this, rootFd, PROCDIR, NULL, period, now); + LinuxProcessList_recurseProcTree(this, rootFd, PROCDIR, NULL, period, super->timestampMs); }