From b4a92e396f2ac79c7d46eb6d40031c9ede332fd8 Mon Sep 17 00:00:00 2001 From: Tom Cnops Date: Mon, 5 Sep 2022 15:21:55 +0200 Subject: [PATCH 1/4] Remove logic that increases delay between progress updates over time --- .../build/lib/runtime/UiEventHandler.java | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java index 50aba39681b4a2..417cc8382bad94 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java @@ -81,15 +81,10 @@ public final class UiEventHandler implements EventHandler { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); - /** Latest refresh of the progress bar, if contents other than time changed */ - private static final long MAXIMAL_UPDATE_DELAY_MILLIS = 200L; + /** Minimal time between scheduled updates */ + private static final long MINIMAL_UPDATE_INTERVAL_MILLIS = 200L; /** Minimal rate limiting (in ms), if the progress bar cannot be updated in place */ private static final long NO_CURSES_MINIMAL_PROGRESS_RATE_LIMIT = 1000L; - /** - * Minimal rate limiting, as fraction of the request time so far, if the progress bar cannot be - * updated in place - */ - private static final double NO_CURSES_MINIMAL_RELATIVE_PROGRESS_RATE_LMIT = 0.15; /** Periodic update interval of a time-dependent progress bar if it can be updated in place */ private static final long SHORT_REFRESH_MILLIS = 1000L; /** Periodic update interval of a time-dependent progress bar if it cannot be updated in place */ @@ -101,7 +96,6 @@ public final class UiEventHandler implements EventHandler { private final boolean cursorControl; private final Clock clock; - private final long uiStartTimeMillis; private final AnsiTerminal terminal; private final boolean debugAllEvents; private final UiStateTracker stateTracker; @@ -175,7 +169,6 @@ public UiEventHandler( this.progressInTermTitle = options.progressInTermTitle && options.useCursorControl(); this.showTimestamp = options.showTimestamp; this.clock = clock; - this.uiStartTimeMillis = clock.currentTimeMillis(); this.debugAllEvents = options.experimentalUiDebugAllEvents; this.locationPrinter = new LocationPrinter(options.attemptToPrintRelativePaths, workspacePathFragment); @@ -207,7 +200,7 @@ public UiEventHandler( Math.round(options.showProgressRateLimit * 1000), NO_CURSES_MINIMAL_PROGRESS_RATE_LIMIT); } - this.minimalUpdateInterval = Math.max(this.minimalDelayMillis, MAXIMAL_UPDATE_DELAY_MILLIS); + this.minimalUpdateInterval = Math.max(this.minimalDelayMillis, MINIMAL_UPDATE_INTERVAL_MILLIS); this.stdoutLineBuffer = new ByteArrayOutputStream(); this.stderrLineBuffer = new ByteArrayOutputStream(); this.dateShown = false; @@ -861,17 +854,6 @@ private void doRefresh(boolean fromUpdateThread) { clearProgressBar(); addProgressBar(); terminal.flush(); - if (!cursorControl) { - // If we can't update the progress bar in place, make sure we increase the update - // interval as time progresses, to avoid too many progress messages in place. - minimalDelayMillis = - Math.max( - minimalDelayMillis, - Math.round( - NO_CURSES_MINIMAL_RELATIVE_PROGRESS_RATE_LMIT - * (clock.currentTimeMillis() - uiStartTimeMillis))); - minimalUpdateInterval = Math.max(minimalDelayMillis, MAXIMAL_UPDATE_DELAY_MILLIS); - } } } } catch (IOException e) { From e3f4fb86e9da82f7257299ecac9f03a30ee08b23 Mon Sep 17 00:00:00 2001 From: Tom Cnops Date: Tue, 18 Oct 2022 15:05:35 +0200 Subject: [PATCH 2/4] Update docs to clarify progress_report_interval and show_progress_rate_limit --- site/en/docs/user-manual.md | 12 +++++++----- .../build/lib/buildtool/BuildRequestOptions.java | 6 ++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/site/en/docs/user-manual.md b/site/en/docs/user-manual.md index 65964d00df4d55..fa63b8ad5a50d4 100644 --- a/site/en/docs/user-manual.md +++ b/site/en/docs/user-manual.md @@ -799,6 +799,9 @@ The default is 0, that means an incremental algorithm: the first report will be printed after 10 seconds, then 30 seconds and after that progress is reported once every minute. +When bazel is using cursor control, as specified by +[`--curses`](#curses), progress is reported every second. + #### `--local_{ram,cpu}_resources {{ "" }}resources or resource expression{{ "" }}` {:#local-resources} These options specify the amount of local resources (RAM in MB and number of CPU logical cores) @@ -1135,11 +1138,10 @@ default. When disabled, progress messages are suppressed. #### `--show_progress_rate_limit={{ "" }}n{{ "" }}` {:#show-progress-rate} -This option causes bazel to display only -one progress message per `n` seconds, where {{ "" }}n{{ "" }} is a real number. -If `n` is -1, all progress messages will be displayed. The default value for -this option is 0.02, meaning bazel will limit the progress messages to one per every -0.02 seconds. +This option causes bazel to display at most one progress message per `n` seconds, +where {{ "" }}n{{ "" }} is a real number. +The default value for this option is 0.02, meaning bazel will limit the progress +messages to one per every 0.02 seconds. #### `--show_result={{ "" }}n{{ "" }}` {:#show-result} diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index 6cfb3cbf416132..9957c81a46ed7d 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java @@ -73,8 +73,10 @@ public class BuildRequestOptions extends OptionsBase { effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, converter = ProgressReportIntervalConverter.class, help = - "The number of seconds to wait between two reports on still running jobs. The " - + "default value 0 means to use the default 10:30:60 incremental algorithm.") + "The number of seconds to between reports on still running jobs. The " + + "default value 0 means the first report will be printed after 10 " + + "seconds, then 30 seconds and after that progress is reported once every minute. " + + "When --curses is enabled, progress is reported every second.") public int progressReportInterval; @Option( From fee6aae4194c7a37599cec3937a821f7b39bd22d Mon Sep 17 00:00:00 2001 From: Tom Cnops Date: Tue, 18 Oct 2022 15:06:26 +0200 Subject: [PATCH 3/4] Do not do time-based updates without cursor control --- .../devtools/build/lib/runtime/UiEventHandler.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java index 417cc8382bad94..b28ee9d97a8e17 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java @@ -87,8 +87,6 @@ public final class UiEventHandler implements EventHandler { private static final long NO_CURSES_MINIMAL_PROGRESS_RATE_LIMIT = 1000L; /** Periodic update interval of a time-dependent progress bar if it can be updated in place */ private static final long SHORT_REFRESH_MILLIS = 1000L; - /** Periodic update interval of a time-dependent progress bar if it cannot be updated in place */ - private static final long LONG_REFRESH_MILLIS = 20000L; private static final DateTimeFormatter TIMESTAMP_FORMAT = DateTimeFormatter.ofPattern("(HH:mm:ss) "); @@ -881,7 +879,7 @@ private void refreshSoon() { // a future update scheduled. long nowMillis = clock.currentTimeMillis(); if (mustRefreshAfterMillis <= lastRefreshMillis) { - mustRefreshAfterMillis = Math.max(nowMillis + minimalUpdateInterval, lastRefreshMillis + 1); + mustRefreshAfterMillis = Math.max(nowMillis + 1, lastRefreshMillis + minimalUpdateInterval); } startUpdateThread(); } @@ -891,16 +889,19 @@ private synchronized boolean timeBasedRefresh() { if (!stateTracker.progressBarTimeDependent()) { return false; } + // Don't do more updates than are requested through events when there is no cursor control. + if (!cursorControl){ + return false; + } long nowMillis = clock.currentTimeMillis(); - long intervalMillis = cursorControl ? SHORT_REFRESH_MILLIS : LONG_REFRESH_MILLIS; if (lastRefreshMillis < mustRefreshAfterMillis && mustRefreshAfterMillis < nowMillis + minimalDelayMillis) { - // Within the a smal interval from now, an update is scheduled anyway, + // Within a small interval from now, an update is scheduled anyway, // so don't do a time-based update of the progress bar now, to avoid // updates too close to each other. return false; } - return lastRefreshMillis + intervalMillis < nowMillis; + return lastRefreshMillis + SHORT_REFRESH_MILLIS < nowMillis; } private void ignoreRefreshLimitOnce() { From 2c1d453b574ed7c655211827bf0f878a483a9d48 Mon Sep 17 00:00:00 2001 From: Tom Cnops Date: Wed, 26 Oct 2022 18:11:54 +0200 Subject: [PATCH 4/4] Rename variable --- .../build/lib/runtime/UiEventHandler.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java index b28ee9d97a8e17..4d26632ed21e3c 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java @@ -103,7 +103,7 @@ public final class UiEventHandler implements EventHandler { private final boolean showTimestamp; private final OutErr outErr; private final ImmutableSet filteredEvents; - private long minimalDelayMillis; + private long progressRateLimitMillis; private long minimalUpdateInterval; private long lastRefreshMillis; private long mustRefreshAfterMillis; @@ -191,14 +191,16 @@ public UiEventHandler( this.stateTracker.setProgressSampleSize(options.uiActionsShown); this.numLinesProgressBar = 0; if (this.cursorControl) { - this.minimalDelayMillis = Math.round(options.showProgressRateLimit * 1000); + this.progressRateLimitMillis = Math.round(options.showProgressRateLimit * 1000); } else { - this.minimalDelayMillis = + this.progressRateLimitMillis = Math.max( Math.round(options.showProgressRateLimit * 1000), NO_CURSES_MINIMAL_PROGRESS_RATE_LIMIT); } - this.minimalUpdateInterval = Math.max(this.minimalDelayMillis, MINIMAL_UPDATE_INTERVAL_MILLIS); + this.minimalUpdateInterval = Math.max( + this.progressRateLimitMillis, + MINIMAL_UPDATE_INTERVAL_MILLIS); this.stdoutLineBuffer = new ByteArrayOutputStream(); this.stderrLineBuffer = new ByteArrayOutputStream(); this.dateShown = false; @@ -843,7 +845,7 @@ private void doRefresh(boolean fromUpdateThread) { return; } long nowMillis = clock.currentTimeMillis(); - if (lastRefreshMillis + minimalDelayMillis < nowMillis) { + if (lastRefreshMillis + progressRateLimitMillis < nowMillis) { if (updateLock.tryLock()) { try { synchronized (this) { @@ -895,7 +897,7 @@ private synchronized boolean timeBasedRefresh() { } long nowMillis = clock.currentTimeMillis(); if (lastRefreshMillis < mustRefreshAfterMillis - && mustRefreshAfterMillis < nowMillis + minimalDelayMillis) { + && mustRefreshAfterMillis < nowMillis + progressRateLimitMillis) { // Within a small interval from now, an update is scheduled anyway, // so don't do a time-based update of the progress bar now, to avoid // updates too close to each other. @@ -907,7 +909,7 @@ private synchronized boolean timeBasedRefresh() { private void ignoreRefreshLimitOnce() { // Set refresh time variables in a state such that the next progress bar // update will definitely be written out. - lastRefreshMillis = clock.currentTimeMillis() - minimalDelayMillis - 1; + lastRefreshMillis = clock.currentTimeMillis() - progressRateLimitMillis - 1; } private void startUpdateThread() {