From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2009) id CFF863865C0E; Thu, 10 Nov 2022 17:03:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CFF863865C0E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668099839; bh=r3TOK1GIR5H2FR0/sb0U0B6vfgoVRsvliau3UKvblTs=; h=From:To:Subject:Date:From; b=CvsEvOYHvo3YHFrl0nB4OxWKT9+RTcEBnm/ZBTW2nZ8Yw3NiN1LzMysspTOTxHDma jjJmsxfbnk3STGSJHr7o5Txt6ykWnhDDU7YIb35qyICRdaKPyI+xlsjOjw1pXVR99E Ra7LGyhii8zkILa79qYpgFKUn9l74CWnTmS4NZPc= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Aaron Merey To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/debuginfod: Improve progress updates X-Act-Checkin: binutils-gdb X-Git-Author: Aaron Merey X-Git-Refname: refs/heads/master X-Git-Oldrev: f71e3f86e83c9c22e3d86112b5ddb61919390a1a X-Git-Newrev: 27859c6b9d73a8ae1b51c5c40fc2b3aefd2228a0 Message-Id: <20221110170359.CFF863865C0E@sourceware.org> Date: Thu, 10 Nov 2022 17:03:59 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D27859c6b9d73= a8ae1b51c5c40fc2b3aefd2228a0 commit 27859c6b9d73a8ae1b51c5c40fc2b3aefd2228a0 Author: Aaron Merey Date: Mon Oct 24 14:05:06 2022 -0400 gdb/debuginfod: Improve progress updates =20 If the download size is known, a progress bar is displayed along with the percentage of completion and the total download size. =20 Downloading separate debug info for /lib/libxyz.so [############ ] 25% (10.01 M) =20 If the download size is not known, a progress indicator is displayed with a ticker ("###") that moves across the screen at a rate of 1 tick every 0.5 seconds. =20 Downloading separate debug info for /lib/libxyz.so [ ### ] =20 If the output stream is not a tty, batch mode is enabled, the screen is too narrow or width has been set to 'unlimited', then only a static description of the download is printed. No bar or ticker is displayed. =20 Downloading separate debug info for /lib/libxyz.so... =20 In any case, if the size of the download is known at the time the description is printed then it will be included in the description. =20 Downloading 10.01 MB separate debug info for /lib/libxyz.so... Diff: --- gdb/cli-out.c | 189 ++++++++++++++++++++++++++++++-------------= ---- gdb/cli-out.h | 40 +++++----- gdb/debuginfod-support.c | 116 ++++++++++++++++++----------- gdb/mi/mi-out.c | 32 ++++++++ gdb/mi/mi-out.h | 28 ++++--- gdb/ui-out.h | 53 ++++++++----- 6 files changed, 296 insertions(+), 162 deletions(-) diff --git a/gdb/cli-out.c b/gdb/cli-out.c index fdbed6f5e91..9c2f0183031 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -26,6 +26,7 @@ #include "completer.h" #include "readline/readline.h" #include "cli/cli-style.h" +#include "top.h" =20 /* These are the CLI output functions */ =20 @@ -262,105 +263,157 @@ cli_ui_out::do_redirect (ui_file *outstream) m_streams.pop_back (); } =20 -/* The cli_ui_out::do_progress_* functions result in the following: - - printed for tty, SHOULD_PRINT =3D=3D true: - - - printed for tty, SHOULD_PRINT =3D=3D false: - <> +/* Initialize a progress update to be displayed with + cli_ui_out::do_progress_notify. */ + +void +cli_ui_out::do_progress_start () +{ + m_progress_info.emplace_back (); +} + +#define MIN_CHARS_PER_LINE 50 +#define MAX_CHARS_PER_LINE 4096 + +/* Print a progress update. MSG is a string to be printed on the line abo= ve + the progress bar. TOTAL is the size of the download whose progress is + being displayed. UNIT should be the unit of TOTAL (ex. "K"). If HOWMUCH + is between 0.0 and 1.0, a progress bar is displayed indicating the perc= entage + of completion and the download size. If HOWMUCH is negative, a progress + indicator will tick across the screen. If the output stream is not a t= ty + then only MSG is printed. + + - printed for tty, HOWMUCH between 0.0 and 1.0: + + - printed for tty, HOWMUCH < 0.0: + - printed for not-a-tty: - + */ =20 void -cli_ui_out::do_progress_start (const std::string &name, bool should_print) +cli_ui_out::do_progress_notify (const std::string &msg, + const char *unit, + double howmuch, double total) { + int chars_per_line =3D get_chars_per_line (); struct ui_file *stream =3D m_streams.back (); - cli_progress_info meter; + cli_progress_info &info (m_progress_info.back ()); + + if (chars_per_line > MAX_CHARS_PER_LINE) + chars_per_line =3D MAX_CHARS_PER_LINE; + + if (info.state =3D=3D progress_update::START) + { + if (stream->isatty () + && current_ui->input_interactive_p () + && chars_per_line >=3D MIN_CHARS_PER_LINE) + { + gdb_printf (stream, "%s\n", msg.c_str ()); + info.state =3D progress_update::BAR; + } + else + { + gdb_printf (stream, "%s...\n", msg.c_str ()); + info.state =3D progress_update::WORKING; + } + } + + if (info.state !=3D progress_update::BAR + || chars_per_line < MIN_CHARS_PER_LINE) + return; =20 - meter.last_value =3D 0; - meter.name =3D name; - if (!stream->isatty ()) + if (total > 0 && howmuch >=3D 0 && howmuch <=3D 1.0) { - gdb_printf (stream, "%s...", meter.name.c_str ()); + std::string progress =3D string_printf (" %3.f%% (%.2f %s)", + howmuch * 100, total, + unit); + int width =3D chars_per_line - progress.size () - 4; + int max =3D width * howmuch; + + std::string display =3D "\r["; + + for (int i =3D 0; i < width; ++i) + if (i < max) + display +=3D "#"; + else + display +=3D " "; + + display +=3D "]" + progress; + gdb_printf (stream, "%s", display.c_str ()); gdb_flush (stream); - meter.printing =3D WORKING; } else { - /* Don't actually emit anything until the first call notifies us - of progress. This makes it so a second progress message can - be started before the first one has been notified, without - messy output. */ - meter.printing =3D should_print ? START : NO_PRINT; + using namespace std::chrono; + milliseconds diff =3D duration_cast + (steady_clock::now () - info.last_update); + + /* Advance the progress indicator at a rate of 1 tick every + every 0.5 seconds. */ + if (diff.count () >=3D 500) + { + int width =3D chars_per_line - 4; + + gdb_printf (stream, "\r["); + for (int i =3D 0; i < width; ++i) + { + if (i =3D=3D info.pos % width + || i =3D=3D (info.pos + 1) % width + || i =3D=3D (info.pos + 2) % width) + gdb_printf (stream, "#"); + else + gdb_printf (stream, " "); + } + + gdb_printf (stream, "]"); + gdb_flush (stream); + info.last_update =3D steady_clock::now (); + info.pos++; + } } =20 - m_meters.push_back (std::move (meter)); + return; } =20 +/* Clear the current line of the most recent progress update. Overwrites + the current line with whitespace. */ + void -cli_ui_out::do_progress_notify (double howmuch) +cli_ui_out::clear_current_line () { struct ui_file *stream =3D m_streams.back (); - cli_progress_info &meter (m_meters.back ()); + int chars_per_line =3D get_chars_per_line (); =20 - if (meter.printing =3D=3D NO_PRINT) + if (!stream->isatty () + || !current_ui->input_interactive_p () + || chars_per_line < MIN_CHARS_PER_LINE) return; =20 - if (meter.printing =3D=3D START) - { - gdb_printf (stream, "%s\n", meter.name.c_str ()); - gdb_flush (stream); - meter.printing =3D WORKING; - } - - if (meter.printing =3D=3D WORKING && howmuch >=3D 1.0) - return; + if (chars_per_line > MAX_CHARS_PER_LINE) + chars_per_line =3D MAX_CHARS_PER_LINE; =20 - if (!stream->isatty ()) - return; + gdb_printf (stream, "\r"); + for (int i =3D 0; i < chars_per_line; ++i) + gdb_printf (stream, " "); + gdb_printf (stream, "\r"); =20 - int chars_per_line =3D get_chars_per_line (); - if (chars_per_line > 0) - { - int i, max; - int width =3D chars_per_line - 3; - - max =3D width * howmuch; - gdb_printf (stream, "\r["); - for (i =3D 0; i < width; ++i) - gdb_printf (stream, i < max ? "#" : " "); - gdb_printf (stream, "]"); - gdb_flush (stream); - meter.printing =3D PROGRESS; - } + gdb_flush (stream); } =20 +/* Remove the most recent progress update from the progress_info stack + and overwrite the current line with whitespace. */ + void cli_ui_out::do_progress_end () { struct ui_file *stream =3D m_streams.back (); - cli_progress_info &meter =3D m_meters.back (); - - if (!stream->isatty ()) - { - gdb_printf (stream, "\n"); - gdb_flush (stream); - } - else if (meter.printing =3D=3D PROGRESS) - { - int i; - int width =3D get_chars_per_line () - 3; - - gdb_printf (stream, "\r"); - for (i =3D 0; i < width + 2; ++i) - gdb_printf (stream, " "); - gdb_printf (stream, "\r"); - gdb_flush (stream); - } + m_progress_info.pop_back (); =20 - m_meters.pop_back (); + if (stream->isatty ()) + clear_current_line (); } =20 /* local functions */ diff --git a/gdb/cli-out.h b/gdb/cli-out.h index 3f01fe0db6d..06d155f7779 100644 --- a/gdb/cli-out.h +++ b/gdb/cli-out.h @@ -21,6 +21,7 @@ #define CLI_OUT_H =20 #include "ui-out.h" +#include #include =20 class cli_ui_out : public ui_out @@ -71,8 +72,9 @@ protected: virtual void do_flush () override; virtual void do_redirect (struct ui_file *outstream) override; =20 - virtual void do_progress_start (const std::string &, bool) override; - virtual void do_progress_notify (double) override; + virtual void do_progress_start () override; + virtual void do_progress_notify (const std::string &, const char *, + double, double) override; virtual void do_progress_end () override; =20 bool suppress_output () @@ -85,32 +87,24 @@ private: std::vector m_streams; bool m_suppress_output; =20 - /* Represents the printing state of a progress meter. */ - enum meter_state - { - /* Printing will start with the next output. */ - START, - /* Printing has already started. */ - WORKING, - /* Progress printing has already started. */ - PROGRESS, - /* Printing should not be done. */ - NO_PRINT - }; - - /* The state of a recent progress meter. */ + /* The state of a recent progress update. */ struct cli_progress_info { + /* Position of the progress indicator. */ + int pos; /* The current state. */ - enum meter_state printing; - /* The name to print. */ - std::string name; - /* The last notification value. */ - double last_value; + progress_update::state state; + /* Progress indicator's time of last update. */ + std::chrono::steady_clock::time_point last_update; + + cli_progress_info () + : pos (0), state (progress_update::START) + {} }; =20 - /* Stack of progress meters. */ - std::vector m_meters; + /* Stack of progress info. */ + std::vector m_progress_info; + void clear_current_line (); }; =20 extern void cli_display_match_list (char **matches, int len, int max); diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c index 5f04a2b38ca..f1249203da0 100644 --- a/gdb/debuginfod-support.c +++ b/gdb/debuginfod-support.c @@ -24,6 +24,7 @@ #include "gdbsupport/gdb_optional.h" #include "cli/cli-cmds.h" #include "cli/cli-style.h" +#include "cli-out.h" #include "target.h" =20 /* Set/show debuginfod commands. */ @@ -87,12 +88,12 @@ debuginfod_exec_query (const unsigned char *build_id, struct user_data { user_data (const char *desc, const char *fname) - : desc (desc), fname (fname), has_printed (false) + : desc (desc), fname (fname) { } =20 const char * const desc; const char * const fname; - bool has_printed; + ui_out::progress_update progress; }; =20 /* Deleter for a debuginfod_client. */ @@ -108,47 +109,74 @@ struct debuginfod_client_deleter using debuginfod_client_up =3D std::unique_ptr; =20 + +/* Convert SIZE into a unit suitable for use with progress updates. + SIZE should in given in bytes and will be converted into KB, MB, GB + or remain unchanged. UNIT will be set to "B", "KB", "MB" or "GB" + accordingly. */ + +static const char * +get_size_and_unit (double &size) +{ + if (size < 1024) + /* If size is less than 1 KB then set unit to B. */ + return "B"; + + size /=3D 1024; + if (size < 1024) + /* If size is less than 1 MB then set unit to KB. */ + return "K"; + + size /=3D 1024; + if (size < 1024) + /* If size is less than 1 GB then set unit to MB. */ + return "M"; + + size /=3D 1024; + return "G"; +} + static int progressfn (debuginfod_client *c, long cur, long total) { user_data *data =3D static_cast (debuginfod_get_user_data (= c)); gdb_assert (data !=3D nullptr); =20 + string_file styled_fname (current_uiout->can_emit_style_escape ()); + fprintf_styled (&styled_fname, file_name_style.style (), "%s", + data->fname); + if (check_quit_flag ()) { - gdb_printf ("Cancelling download of %s %ps...\n", - data->desc, - styled_string (file_name_style.style (), data->fname)); + gdb_printf ("Cancelling download of %s %s...\n", + data->desc, styled_fname.c_str ()); return 1; } =20 - if (!data->has_printed) + if (debuginfod_verbose =3D=3D 0) + return 0; + + /* Print progress update. Include the transfer size if available. */ + if (total > 0) { - /* Include the transfer size, if available. */ - if (total > 0) + /* Transfer size is known. */ + double howmuch =3D (double) cur / (double) total; + + if (howmuch >=3D 0.0 && howmuch <=3D 1.0) { - float size =3D 1.0f * total / 1024; - const char *unit =3D "KB"; - - /* If size is greater than 0.01 MB, set unit to MB. */ - if (size > 10.24) - { - size /=3D 1024; - unit =3D "MB"; - } - - gdb_printf ("Downloading %.2f %s %s %ps...\n", - size, unit, data->desc, - styled_string (file_name_style.style (), - data->fname)); + double d_total =3D (double) total; + const char *unit =3D get_size_and_unit (d_total); + std::string msg =3D string_printf ("Downloading %0.2f %s %s %s", + d_total, unit, data->desc, + styled_fname.c_str ()); + data->progress.update_progress (msg, unit, howmuch, d_total); + return 0; } - else - gdb_printf ("Downloading %s %ps...\n", data->desc, - styled_string (file_name_style.style (), data->fname)); - - data->has_printed =3D true; } =20 + std::string msg =3D string_printf ("Downloading %s %s", + data->desc, styled_fname.c_str ()); + data->progress.update_progress (msg); return 0; } =20 @@ -230,6 +258,21 @@ debuginfod_is_enabled () return true; } =20 +/* Print the result of the most recent attempted download. */ + +static void +print_outcome (user_data &data, int fd) +{ + /* Clears the current line of progress output. */ + current_uiout->do_progress_end (); + + if (fd < 0 && fd !=3D -ENOENT) + gdb_printf (_("Download failed: %s. Continuing without %s %ps.\n"), + safe_strerror (-fd), + data.desc, + styled_string (file_name_style.style (), data.fname)); +} + /* See debuginfod-support.h */ =20 scoped_fd @@ -263,11 +306,7 @@ debuginfod_source_query (const unsigned char *build_id, srcpath, &dname)); debuginfod_set_user_data (c, nullptr); - - if (fd.get () < 0 && fd.get () !=3D -ENOENT) - gdb_printf (_("Download failed: %s. Continuing without source file %p= s.\n"), - safe_strerror (-fd.get ()), - styled_string (file_name_style.style (), srcpath)); + print_outcome (data, fd.get ()); =20 if (fd.get () >=3D 0) destname->reset (dname); @@ -305,11 +344,7 @@ debuginfod_debuginfo_query (const unsigned char *build= _id, scoped_fd fd (debuginfod_find_debuginfo (c, build_id, build_id_len, &dname)); debuginfod_set_user_data (c, nullptr); - - if (fd.get () < 0 && fd.get () !=3D -ENOENT) - gdb_printf (_("Download failed: %s. Continuing without debug info for= %ps.\n"), - safe_strerror (-fd.get ()), - styled_string (file_name_style.style (), filename)); + print_outcome (data, fd.get ()); =20 if (fd.get () >=3D 0) destname->reset (dname); @@ -346,12 +381,7 @@ debuginfod_exec_query (const unsigned char *build_id, =20 scoped_fd fd (debuginfod_find_executable (c, build_id, build_id_len, &dn= ame)); debuginfod_set_user_data (c, nullptr); - - if (fd.get () < 0 && fd.get () !=3D -ENOENT) - gdb_printf (_("Download failed: %s. " \ - "Continuing without executable for %ps.\n"), - safe_strerror (-fd.get ()), - styled_string (file_name_style.style (), filename)); + print_outcome (data, fd.get ()); =20 if (fd.get () >=3D 0) destname->reset (dname); diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c index 028c0058e58..725c1c68546 100644 --- a/gdb/mi/mi-out.c +++ b/gdb/mi/mi-out.c @@ -259,6 +259,38 @@ mi_ui_out::main_stream () return (string_file *) m_streams.back (); } =20 +/* Initialize a progress update to be displayed with + mi_ui_out::do_progress_notify. */ + +void +mi_ui_out::do_progress_start () +{ + m_progress_info.emplace_back (); +} + +/* Indicate that a task described by MSG is in progress. */ + +void +mi_ui_out::do_progress_notify (const std::string &msg, const char *unit, + double cur, double total) +{ + mi_progress_info &info (m_progress_info.back ()); + + if (info.state =3D=3D progress_update::START) + { + gdb_printf ("%s...\n", msg.c_str ()); + info.state =3D progress_update::WORKING; + } +} + +/* Remove the most recent progress update from the progress_info stack. */ + +void +mi_ui_out::do_progress_end () +{ + m_progress_info.pop_back (); +} + /* Clear the buffer. */ =20 void diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h index 36d7e42345f..d51d8101047 100644 --- a/gdb/mi/mi-out.h +++ b/gdb/mi/mi-out.h @@ -83,17 +83,11 @@ protected: virtual bool do_is_mi_like_p () const override { return true; } =20 - virtual void do_progress_start (const std::string &, bool) override - { - } + virtual void do_progress_start () override; + virtual void do_progress_notify (const std::string &, const char *, + double, double) override; =20 - virtual void do_progress_notify (double) override - { - } - - virtual void do_progress_end () override - { - } + virtual void do_progress_end () override; =20 private: =20 @@ -101,6 +95,20 @@ private: void open (const char *name, ui_out_type type); void close (ui_out_type type); =20 + /* The state of a recent progress_update. */ + struct mi_progress_info + { + /* The current state. */ + progress_update::state state; + + mi_progress_info () + : state (progress_update::START) + {} + }; + + /* Stack of progress info. */ + std::vector m_progress_info; + /* Convenience method that returns the MI out's string stream cast to its appropriate type. Assumes/asserts that output was not redirected. */ diff --git a/gdb/ui-out.h b/gdb/ui-out.h index 65d40875926..5390c23fe79 100644 --- a/gdb/ui-out.h +++ b/gdb/ui-out.h @@ -278,39 +278,56 @@ class ui_out escapes. */ virtual bool can_emit_style_escape () const =3D 0; =20 - /* An object that starts and finishes a progress meter. */ - class progress_meter + /* An object that starts and finishes displaying progress updates. */ + class progress_update { public: + /* Represents the printing state of a progress update. */ + enum state + { + /* Printing will start with the next update. */ + START, + /* Printing has already started. */ + WORKING, + /* Progress bar printing has already started. */ + BAR + }; + /* SHOULD_PRINT indicates whether something should be printed for a tt= y. */ - progress_meter (struct ui_out *uiout, const std::string &name, - bool should_print) - : m_uiout (uiout) + progress_update () { - m_uiout->do_progress_start (name, should_print); + m_uiout =3D current_uiout; + m_uiout->do_progress_start (); } =20 - ~progress_meter () + ~progress_update () { - m_uiout->do_progress_notify (1.0); - m_uiout->do_progress_end (); + } =20 - progress_meter (const progress_meter &) =3D delete; - progress_meter &operator=3D (const progress_meter &) =3D delete; + progress_update (const progress_update &) =3D delete; + progress_update &operator=3D (const progress_update &) =3D delete; =20 - /* Emit some progress for this progress meter. HOWMUCH may range - from 0.0 to 1.0. */ - void progress (double howmuch) + /* Emit some progress for this progress meter. Includes current + amount of progress made and total amount in the display. */ + void update_progress (const std::string& msg, const char *unit, + double cur, double total) { - m_uiout->do_progress_notify (howmuch); + m_uiout->do_progress_notify (msg, unit, cur, total); } =20 + /* Emit some progress for this progress meter. */ + void update_progress (const std::string& msg) + { + m_uiout->do_progress_notify (msg, "", -1, -1); + } private: =20 struct ui_out *m_uiout; }; =20 + virtual void do_progress_end () =3D 0; + protected: =20 virtual void do_table_begin (int nbrofcols, int nr_rows, const char *tbl= id) @@ -345,9 +362,9 @@ class ui_out virtual void do_flush () =3D 0; virtual void do_redirect (struct ui_file *outstream) =3D 0; =20 - virtual void do_progress_start (const std::string &, bool) =3D 0; - virtual void do_progress_notify (double) =3D 0; - virtual void do_progress_end () =3D 0; + virtual void do_progress_start () =3D 0; + virtual void do_progress_notify (const std::string &, const char *, + double, double) =3D 0; =20 /* Set as not MI-like by default. It is overridden in subclasses if necessary. */