Highlight new and old processes (#74)

[adsr]

(Hacktoberfest entry.) Relies on starttime_ctime which does not appear to be set in Linux. (The starttime field in /proc/<pid>/stat is not immediately useable.)

[fasterit]

General change requests:

• This should also be a setting in the setup menu
• This should highlight the whole line like the search bar and the current process and not only the length of the text

Please also document the command line flag (preferably -H, see above) in the main page (htop.1.in).

[adsr]

@fasterit @BenBE Thanks for the review. I addressed most of the review comments in cf1f9a7. Remaining work:

• Highlight full line
• Avoid _scanTs global
• Prevent highlighting processes on start-up more elegantly
• Add configurable highlight time

The first three will take some refactoring I think. Pausing here for review.

[BenBE]

What's the difference between the global ProcessList_scanTs and the scanTs value in the ProcessList structure?

[ghost]

Congratulations 🎉. DeepCode analyzed your code in 1.65 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

[adsr]

What's the difference between the global ProcessList_scanTs and the scanTs value in the ProcessList structure?

I removed the global in 7caabfc.

[adsr]

Hello. Can someone add a hacktoberfest-accepted label to this issue so that it counts toward Hacktoberfest?

[fasterit]

@fasterit @BenBE Thanks for the review. I addressed most of the review comments in cf1f9a7. Remaining work:

* Highlight full line
* Avoid _scanTs global
* Prevent highlighting processes on start-up more elegantly
* Add configurable highlight time

The first three will take some refactoring I think. Pausing here for review.

What is the status of these? I.e. why don't you finish the PR so we can merge it?

[adsr]

• Avoid _scanTs global

This is resolved by 7caabfc.

• Add configurable highlight time

I can add this if you'd like.

• Highlight full line
• Prevent highlighting processes on start-up more elegantly

I believe these should be tracked in separate issues as they require heavier refactoring.

[fasterit]

• Add configurable highlight time
  I can add this if you'd like.

Please do.

• Highlight full line
• Prevent highlighting processes on start-up more elegantly

I believe these should be tracked in separate issues as they require heavier refactoring.

That doesn't make sense as we won't merge it as is.
So feel free to push to this branch of yours.

[BenBE]

While testing the branch recently I noticed the highlighted line markup doesn't reach the end of the line. This is kinda irritating. Also with 5 seconds it feels kinda hectic, thus having a configuration for this would be nice.

Apart from this, this PR should be rebased onto current master to resolve the merge conflicts..

[fasterit]

Still working on this @adsr? Need any help?

[adsr]

The flag is now --highlight-changes[=DELAY]. Settings get written to htoprc. Full line gets highlighted.

Also rebased and resolved conflicts.

[fasterit]

Very nice, thank you very much @adsr.

Just as a comment:
As we save --delay values in htoprc and not allow setting them via the setup GUI, this seems fine for --highlight-changes as well.
Medium term we may want a setup screen for these numeric values as well.

[BenBE]

Small issues I noticed:

1. Thombstones are shown for threads even when threads are hidden
2. Minor code issues

Future PRs could also include:

1. Scrolling right on a thumbstone still shows the issue with the incomplete line markup (this seems to be a general issue with h-scrolling though).
2. Would be nice to have a key mapping (C???) to quickly enable/disable at runtime

I maybe found a nice way around your firstScanTs issue: If you manage to get the scanTs for the first process list scan set to 0 you should be fine (htop does one scan which is AFAIR never displayed). Would allow to remove the firstScanTs field.

[adsr]

Thanks for the reviews. a83f515 addresses everything except for removing firstScanTs. @BenBE can you explain the idea again?

[BenBE]

LGTM.

[BenBE]

@BenBE can you explain the idea again?

The idea is basically to use the first process scan of htop (which never gets displayed AFAIR) to set all pre-existing processes to a timestamp far in the past. The first regular scan then uses just the normal timestamps. Each process that existed in the first scan would then have a scanTs so small, that it is considered not changed.

Suggestion for a patch:

diff --git a/ProcessList.c b/ProcessList.c
index 364de1e..8b244df 100644
--- a/ProcessList.c
+++ b/ProcessList.c
@@ -29,7 +29,6 @@ ProcessList* ProcessList_init(ProcessList* this, const ObjectClass* klass, Users
    this->cpuCount = 0;
 
    this->scanTs = 0;
-   this->firstScanTs = 0;
 
 #ifdef HAVE_LIBHWLOC
    this->topologyOk = false;
@@ -87,12 +86,8 @@ void ProcessList_add(ProcessList* this, Process* p) {
    assert(Hashtable_get(this->processTable, p->pid) == NULL);
    p->processList = this;
 
-   if (this->scanTs == this->firstScanTs) {
-      // prevent highlighting processes found in first scan
-      p->seenTs = this->firstScanTs - this->settings->highlightDelaySecs - 1;
-   } else {
-      p->seenTs = this->scanTs;
-   }
+   // highlighting processes found in first scan by first scan marked "far in the past"
+   p->seenTs = this->scanTs;
 
    Vector_add(this->processes, p);
    Hashtable_put(this->processTable, p->pid, p);
@@ -318,10 +313,11 @@ void ProcessList_scan(ProcessList* this, bool pauseProcessUpdate) {
 
 
    // set scanTs
-   if (clock_gettime(CLOCK_MONOTONIC, &now) == 0) {
-      if (this->firstScanTs == 0) {
-         this->firstScanTs = now.tv_sec;
-      }
+   static bool firstScanDone = false;
+   if (!firstScanDone) {
+      this->scanTs = 0;
+      firstScanDone = true;
+   } else if (clock_gettime(CLOCK_MONOTONIC, &now) == 0) {
       this->scanTs = now.tv_sec;
    }
 
diff --git a/ProcessList.h b/ProcessList.h
index db0549a..3e35130 100644
--- a/ProcessList.h
+++ b/ProcessList.h
@@ -71,7 +71,6 @@ typedef struct ProcessList_ {
    int cpuCount;
 
    time_t scanTs;
-   time_t firstScanTs;
 } ProcessList;
 
 ProcessList* ProcessList_new(UsersTable* usersTable, Hashtable* pidMatchList, uid_t userId);