[PATCH] debuginfod-find: Be a bit less verbose with -v

[PATCH] debuginfod-find: Be a bit less verbose with -v

[Mark Wielaard]

debuginfod-find -v enables a progressfn that prints the Progress every
time the callback is called. For slow transfers or big downloads this
can be really verbose (hundreds a times a second). Slow it down a bit,
so it only prints the progress at most 5 times a second.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 debuginfod/ChangeLog         |  5 +++++
 debuginfod/debuginfod-find.c | 24 +++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index a02643e1..d4face2d 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-11-11  Mark Wielaard  <mark@klomp.org>
+
+	* debuginfod-find.c (progressfn): Use clock_gettime to print Progress
+	at most 5 times a second.
+
 2020-11-01  Érico N. Rolim  <erico.erc@gmail.com>
 
 	* debuginfod-client.c (debuginfod_init_cache): Use ACCESSPERMS for
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 88a460f8..5f9bccc3 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -24,6 +24,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 #include <argp.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -64,7 +65,28 @@ static int verbose;
 int progressfn(debuginfod_client *c __attribute__((__unused__)),
 	       long a, long b)
 {
-  fprintf (stderr, "Progress %ld / %ld\n", a, b);
+  static bool first = true;
+  static struct timespec last;
+  struct timespec now;
+  uint64_t delta;
+  if (!first)
+    {
+      clock_gettime (CLOCK_MONOTONIC, &now);
+      delta = ((now.tv_sec - last.tv_sec) * 1000000
+	       + (now.tv_nsec - last.tv_nsec) / 1000);
+    }
+  else
+    {
+      first = false;
+      delta = 250000;
+    }
+
+  /* Show progress the first time and then at most 5 times a second. */
+  if (delta > 200000)
+    {
+      fprintf (stderr, "Progress %ld / %ld\n", a, b);
+      clock_gettime (CLOCK_MONOTONIC, &last);
+    }
   return 0;
 }
 
-- 
2.18.4

Re: [PATCH] debuginfod-find: Be a bit less verbose with -v

[Frank Ch. Eigler]

Hi -

On Wed, Nov 11, 2020 at 09:31:38PM +0100, Mark Wielaard wrote:
> debuginfod-find -v enables a progressfn that prints the Progress every
> time the callback is called. [...]
> [...]
> -  fprintf (stderr, "Progress %ld / %ld\n", a, b);
> [...]

Another option is to use something close what the builtin env
DEBUGINFOD_PROGRESS=1 code does: print self-overwriting messages with
\r rather than \n.  That way many messages can come, but they don't
overpower the screen.  Really, the main reason I put in this
progressfn into debuginfod-find was to help test that API within the
testsuite.  Maybe now, we don't need that option to do anything but
set the env var, therby using the common code.

- FChE

Re: [PATCH] debuginfod-find: Be a bit less verbose with -v

[Mark Wielaard]

Hi Frank,

On Wed, 2020-11-11 at 15:57 -0500, Frank Ch. Eigler wrote:
> On Wed, Nov 11, 2020 at 09:31:38PM +0100, Mark Wielaard wrote:
> > debuginfod-find -v enables a progressfn that prints the Progress
> > every
> > time the callback is called. [...]
> > [...]
> > -  fprintf (stderr, "Progress %ld / %ld\n", a, b);
> > [...]
> 
> Another option is to use something close what the builtin env
> DEBUGINFOD_PROGRESS=1 code does: print self-overwriting messages with
> \r rather than \n.  That way many messages can come, but they don't
> overpower the screen.  Really, the main reason I put in this
> progressfn into debuginfod-find was to help test that API within the
> testsuite.  Maybe now, we don't need that option to do anything but
> set the env var, therby using the common code.

It was indeed the specific testcase that made me keep the messages as
is. And it seems a good idea to have a code path that explicitly uses
the api calls instead of relying on the environment variable. Also I
found it a bit more difficult to combine the self-overwriting messages
with other verbose output. See my followup patch for producing verbose
output. I think the "per line" verbose/progress for debuginfod-find -v
works out well.

Cheers,

Mark

Re: [PATCH] debuginfod-find: Be a bit less verbose with -v

[Mark Wielaard]

On Wed, 2020-11-11 at 22:24 +0100, Mark Wielaard wrote:
> On Wed, 2020-11-11 at 15:57 -0500, Frank Ch. Eigler wrote:
> > On Wed, Nov 11, 2020 at 09:31:38PM +0100, Mark Wielaard wrote:
> > > debuginfod-find -v enables a progressfn that prints the Progress
> > > every
> > > time the callback is called. [...]
> > > [...]
> > > -  fprintf (stderr, "Progress %ld / %ld\n", a, b);
> > > [...]
> > 
> > Another option is to use something close what the builtin env
> > DEBUGINFOD_PROGRESS=1 code does: print self-overwriting messages with
> > \r rather than \n.  That way many messages can come, but they don't
> > overpower the screen.  Really, the main reason I put in this
> > progressfn into debuginfod-find was to help test that API within the
> > testsuite.  Maybe now, we don't need that option to do anything but
> > set the env var, therby using the common code.
> 
> It was indeed the specific testcase that made me keep the messages as
> is. And it seems a good idea to have a code path that explicitly uses
> the api calls instead of relying on the environment variable. Also I
> found it a bit more difficult to combine the self-overwriting messages
> with other verbose output. See my followup patch for producing verbose
> output. I think the "per line" verbose/progress for debuginfod-find -v
> works out well.

Assuming I totally convinced you with the above speech I pushed this
patch. On irc you said you also like the followup
debuginfod_set_verbose_fd/DEBUGINFOD_VERBOSE patch, but I am waiting a
bit longer with that one to see if there is any other feedback (also
because that one exports a new interface).

Cheers,

Mark