progress: show hashing workers as idle when the queue starves

src/file_scan.c

@@ -1876,10 +1876,19 @@ static gpointer scan_worker(gpointer arg)
 	 * actually empty (see pscan_finish_file). Claimed lazily on the first file
 	 * with a non-idle status so a still-unclaimed slot is never reused by a
 	 * sibling worker; a worker that gets no files never shows a phantom line.
+	 * The flip side is that between files the line keeps showing the last one's
+	 * status, so bracket the wait for the next file: a wait long enough to
+	 * outlast a couple of redraws is what makes the line read "idle" instead of
+	 * freezing on the last file's "commit" for the rest of a walk-bound run.
 	 */
 	struct pscan_thread *slot = NULL;
 
-	while ((file = scan_workq_pop(q))) {
+	for (;;) {
+		pscan_slot_waiting(slot, true);
+		file = scan_workq_pop(q);
+		pscan_slot_waiting(slot, false);
+		if (!file)
+			break;
 		if (!slot)
 			slot = pscan_claim_slot(gettid(), thread_scanning);
 		csum_whole_file(file, &buffer, slot);

src/progress.c

@@ -383,6 +383,18 @@ static const char *const BAR_SUB[] = {
 /* Worker-line left column: number(3) + gap(2) + status + gap(2) before the path. */
 #define WORKER_LEFT_W	(3 + 2 + STATUS_COL_W + 2)
 
+/* Redraw cadence of the live block: smooth on a tty, sparse into a log. */
+#define REDRAW_MS	100
+#define REDRAW_LOG_MS	1000
+
+/*
+ * How long a worker may wait for its next unit of work before its line reads
+ * "idle" rather than the status the last one left behind. Longer than a redraw,
+ * so the sub-millisecond gaps between two small files never flicker; short
+ * enough that a starved pool stops lying within a blink.
+ */
+#define IDLE_AFTER_US	(2 * REDRAW_MS * G_TIME_SPAN_MILLISECOND)
+
 /* The dim " · " separator between fields on the bar and detail lines. */
 static void detail_sep(void)
 {
@@ -416,6 +428,20 @@ static const char *status_color(enum pscan_thread_status s)
 	return col_reset;
 }
 
+/*
+ * Does this line read as idle? Either the slot is unclaimed, or its worker has
+ * been waiting for work long enough that drawing the last file's status would
+ * be a lie (a persistent csum worker keeps its line across files, so that
+ * status outlives the file - see pscan_slot_waiting).
+ */
+static bool slot_is_idle(struct pscan_thread *t)
+{
+	gint64 since = t->waiting_since;
+
+	return t->status == thread_idle ||
+	       (since && g_get_monotonic_time() - since > IDLE_AFTER_US);
+}
+
 static void print_thread_progress(struct pscan_thread *t, unsigned int slot)
 {
 	char buf[BUF_LEN];
@@ -426,9 +452,10 @@ static void print_thread_progress(struct pscan_thread *t, unsigned int slot)
 	int avail, termw;
 
 	/* Idle slots are just the dim number and word - nothing on the right. */
-	if (t->status == thread_idle) {
-		s_printf("%s%3u%s  %s%s%s\n",
-			 col_dim, slot, col_reset, wcol, word, col_reset);
+	if (slot_is_idle(t)) {
+		s_printf("%s%3u%s  %s%s%s\n", col_dim, slot, col_reset,
+			 status_color(thread_idle), status_word(thread_idle),
+			 col_reset);
 		return;
 	}
 
@@ -927,7 +954,7 @@ static void *pscan_progress_thread(void * p)
 				json_last = now;
 				json_last_phase = phase;
 			}
-			usleep(100 * 1000);
+			usleep(REDRAW_MS * 1000);
 			continue;
 		}
 
@@ -947,7 +974,7 @@ static void *pscan_progress_thread(void * p)
 		spin_frame++;	/* advance every spinner + the indeterminate bar */
 
 		/* Do not waste too much cpu */
-		usleep(1000 * (tty ? 100 : 1000));
+		usleep(1000 * (tty ? REDRAW_MS : REDRAW_LOG_MS));
 	} while (printer_running);
 
 	return NULL;
@@ -962,10 +989,18 @@ static void pscan_free_threads(void)
 	pscan.thread_count = 0;
 }
 
-struct pscan_thread *pscan_register_thread(pid_t tid)
+/*
+ * Allocate a fresh slot, already claimed by `tid`, and publish it. The status
+ * is set before publication: the moment the slot is in pscan.threads a
+ * concurrent pscan_claim_slot() may look at it, and a still-idle slot there
+ * would be handed to a second worker.
+ */
+static struct pscan_thread *pscan_register_thread(pid_t tid,
+						  enum pscan_thread_status status)
 {
 	struct pscan_thread *tprogress = calloc(1, sizeof(struct pscan_thread));
 	tprogress->tid = tid;
+	tprogress->status = status;
 
 	g_mutex_lock(&pscan.mutex);
 	pscan.threads = realloc(pscan.threads, (pscan.thread_count + 1) *
@@ -1120,9 +1155,18 @@ void pscan_slot_idle(struct pscan_thread *slot)
 	g_mutex_lock(&pscan.mutex);
 	slot->file_path[0] = '\0';
 	slot->status = thread_idle;
+	slot->waiting_since = 0;
 	g_mutex_unlock(&pscan.mutex);
 }
 
+/* See progress.h: the worker publishes the wait, slot_is_idle() judges it. */
+void pscan_slot_waiting(struct pscan_thread *slot, bool waiting)
+{
+	if (!slot)
+		return;
+	slot->waiting_since = waiting ? g_get_monotonic_time() : 0;
+}
+
 bool is_progress_printer_running(void)
 {
 	return printer ? true : false;
@@ -1171,6 +1215,12 @@ struct pscan_thread *pscan_claim_slot(pid_t tid,
 
 	g_mutex_lock(&pscan.mutex);
 	for (unsigned int i = 0; i < pscan.thread_count; i++) {
+		/*
+		 * Only a released slot is free. A persistent worker waiting for
+		 * its next file keeps a non-idle status (that is what
+		 * waiting_since exists to soften, in the renderer only), so its
+		 * line is never handed to a sibling.
+		 */
 		if (pscan.threads[i]->status == thread_idle) {
 			slot = pscan.threads[i];
 			slot->tid = tid;
@@ -1180,10 +1230,8 @@ struct pscan_thread *pscan_claim_slot(pid_t tid,
 	}
 	g_mutex_unlock(&pscan.mutex);
 
-	if (!slot) {
-		slot = pscan_register_thread(tid);
-		slot->status = status;
-	}
+	if (!slot)
+		slot = pscan_register_thread(tid, status);
 	return slot;
 }
 

src/progress.h

@@ -40,6 +40,11 @@ struct pscan_thread {
 	 * the renderer reads it every redraw - hence atomic. The idle/claim
 	 * handoff itself is ordered by the mutex, not by this field. */
 	_Atomic enum pscan_thread_status	status;
+
+	/* Monotonic us at which this slot's worker started waiting for its next
+	 * unit of work, or 0 while it has work. Set by the worker, read by the
+	 * renderer every redraw - hence atomic. See pscan_slot_waiting(). */
+	_Atomic gint64			waiting_since;
 };
 
 struct pscan_global {
@@ -90,9 +95,6 @@ void pscan_set_progress(uint64_t added_files, uint64_t added_bytes);
 void pscan_examined(void);
 uint64_t pscan_files_scanned(void);
 
-/* Used by each scan threads to grab its own struct pscan_thread */
-struct pscan_thread *pscan_register_thread(pid_t tid);
-
 /*
  * Setup the pty and start the progress thread
  * The thread will run until the scan is done, that is:
@@ -128,6 +130,18 @@ void pscan_reset_thread(struct pscan_thread **progress);
 void pscan_finish_file(struct pscan_thread **progress);
 void pscan_slot_idle(struct pscan_thread *slot);
 
+/*
+ * Bracket a persistently-held slot's wait for its next unit of work. Such a
+ * slot keeps showing the last file's status between files, which is right for
+ * the microsecond gap between two small files and a lie once the worker is
+ * starved (the walk is the bottleneck, or nearly everything is up to date).
+ * The worker just publishes when the wait started; the renderer decides when
+ * that has gone on long enough to draw the line as idle instead. One atomic
+ * store (plus a clock read on the `true` side), so it is cheap enough for the
+ * per-file path.
+ */
+void pscan_slot_waiting(struct pscan_thread *slot, bool waiting);
+
 bool is_progress_printer_running(void);
 
 /*

src/tests.c

@@ -350,6 +350,51 @@ MU_TEST(test_scan_workq_priority) {
 	memset(&scan_workq, 0, sizeof(scan_workq));
 }
 
+static gpointer pop_one(gpointer arg)
+{
+	struct file_to_scan **got = arg;
+
+	*got = scan_workq_pop(&scan_workq);
+	return NULL;
+}
+
+MU_TEST(test_starved_worker_line_reads_idle) {
+	/*
+	 * A csum worker holds its display line across files, so the status the
+	 * last file left behind ("commit") is what a starved queue would keep
+	 * showing - for the whole rest of a walk-bound run. The worker publishes
+	 * how long it has been waiting for its next file and the renderer draws
+	 * a long enough wait as idle; a short one (the gap between two small
+	 * files) must still show the file's real status, or the line flickers.
+	 */
+	memset(&scan_workq, 0, sizeof(scan_workq));
+
+	struct file_to_scan file = { .filesize = 4096, .file_position = 1 };
+	struct file_to_scan *got = NULL;
+	struct pscan_thread *slot = pscan_claim_slot(4242, thread_committing);
+
+	/* Waiting on an empty queue: the worker blocks in the pop. */
+	GThread *popper = g_thread_new("pop", pop_one, &got);
+
+	pscan_slot_waiting(slot, true);
+	mu_check(!slot_is_idle(slot));			/* just started waiting */
+	slot->waiting_since -= IDLE_AFTER_US + 1;	/* ... a while ago */
+	mu_check(slot_is_idle(slot));
+
+	/* Still claimed while it waits, so no sibling takes over its line. */
+	mu_check(pscan_claim_slot(4243, thread_scanning) != slot);
+
+	/* Back to work: the line shows the file again, not idle. */
+	scan_workq_push(&file);
+	g_thread_join(popper);
+	pscan_slot_waiting(slot, false);
+	mu_check(got == &file);
+	mu_check(!slot_is_idle(slot));
+
+	pscan_free_threads();
+	memset(&scan_workq, 0, sizeof(scan_workq));
+}
+
 /* Within eps of expected. */
 static bool near(double got, double want, double eps)
 {
@@ -634,6 +679,7 @@ MU_TEST_SUITE(test_suite) {
 	MU_RUN_TEST(test_storage_recommend_io_threads);
 	MU_RUN_TEST(test_scan_bucket);
 	MU_RUN_TEST(test_scan_workq_priority);
+	MU_RUN_TEST(test_starved_worker_line_reads_idle);
 	MU_RUN_TEST(test_scan_eta);
 	MU_RUN_TEST(test_group_u64);
 	MU_RUN_TEST(test_longpath);