Skip to content

Commit 5752762

Browse files
michajlocopybara-github
authored andcommitted
Delete experimental_ui_mode
Skimming the internet and available internal data sources, it doesn't look like it caught on. While it does seem kind of cool/useful, it's also nice to have less knobs to understand/keep in sync/etc. PiperOrigin-RevId: 398540135
1 parent 051d435 commit 5752762

File tree

4 files changed

+7
-100
lines changed

4 files changed

+7
-100
lines changed

src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ public UiEventHandler(
188188
this.cursorControl
189189
? new UiStateTracker(clock, this.terminalWidth - 2)
190190
: new UiStateTracker(clock);
191-
this.stateTracker.setProgressMode(options.uiProgressMode, options.uiSamplesShown);
191+
this.stateTracker.setProgressSampleSize(options.uiActionsShown);
192192
this.numLinesProgressBar = 0;
193193
if (this.cursorControl) {
194194
this.minimalDelayMillis = Math.round(options.showProgressRateLimit * 1000);

src/main/java/com/google/devtools/build/lib/runtime/UiOptions.java

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package com.google.devtools.build.lib.runtime;
1515

1616
import com.google.devtools.build.lib.events.EventKind;
17-
import com.google.devtools.build.lib.runtime.UiStateTracker.ProgressMode;
1817
import com.google.devtools.common.options.Converter;
1918
import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
2019
import com.google.devtools.common.options.Converters.RangeConverter;
@@ -113,13 +112,6 @@ public UseCursesConverter() {
113112
}
114113
}
115114

116-
/** Progress mode converter. */
117-
public static class ProgressModeConverter extends EnumConverter<ProgressMode> {
118-
public ProgressModeConverter() {
119-
super(ProgressMode.class, "--experimental_ui_mode setting");
120-
}
121-
}
122-
123115
@Option(
124116
name = "show_progress",
125117
defaultValue = "true",
@@ -250,19 +242,6 @@ public ProgressModeConverter() {
250242
allowMultiple = true)
251243
public List<EventKind> eventFilters;
252244

253-
@Option(
254-
name = "experimental_ui_mode",
255-
defaultValue = "oldest_actions",
256-
converter = ProgressModeConverter.class,
257-
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
258-
effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
259-
help =
260-
"Determines what kind of data is shown in the detailed progress bar. By default, it is "
261-
+ "set to show the oldest actions and their running time. The underlying data "
262-
+ "source is usually sampled in a mode-dependend way to fit within the number of "
263-
+ "lines given by --ui_actions_shown.")
264-
public ProgressMode uiProgressMode;
265-
266245
@Option(
267246
name = "ui_actions_shown",
268247
oldName = "experimental_ui_actions_shown",
@@ -272,9 +251,8 @@ public ProgressModeConverter() {
272251
help =
273252
"Number of concurrent actions shown in the detailed progress bar; each "
274253
+ "action is shown on a separate line. The progress bar always shows "
275-
+ "at least one one, all numbers less than 1 are mapped to 1. "
276-
+ "This option has no effect if --noui is set.")
277-
public int uiSamplesShown;
254+
+ "at least one one, all numbers less than 1 are mapped to 1.")
255+
public int uiActionsShown;
278256

279257
@Option(
280258
name = "experimental_ui_max_stdouterr_bytes",

src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616
import static com.google.common.base.Preconditions.checkNotNull;
1717

1818
import com.google.common.annotations.VisibleForTesting;
19-
import com.google.common.collect.Comparators;
20-
import com.google.common.collect.HashMultiset;
2119
import com.google.common.collect.Iterables;
22-
import com.google.common.collect.Multiset;
2320
import com.google.common.collect.Sets;
2421
import com.google.devtools.build.lib.actions.Action;
2522
import com.google.devtools.build.lib.actions.ActionCompletionEvent;
@@ -63,7 +60,6 @@
6360
import java.util.Deque;
6461
import java.util.HashSet;
6562
import java.util.LinkedHashMap;
66-
import java.util.List;
6763
import java.util.Map;
6864
import java.util.Optional;
6965
import java.util.PriorityQueue;
@@ -78,11 +74,6 @@
7874
/** Tracks state for the UI. */
7975
final class UiStateTracker {
8076

81-
enum ProgressMode {
82-
OLDEST_ACTIONS,
83-
MNEMONIC_HISTOGRAM
84-
}
85-
8677
private static final long SHOW_TIME_THRESHOLD_SECONDS = 3;
8778
private static final String ELLIPSIS = "...";
8879
private static final String FETCH_PREFIX = " Fetching ";
@@ -93,7 +84,6 @@ enum ProgressMode {
9384
private static final int NANOS_PER_SECOND = 1000000000;
9485
private static final String URL_PROTOCOL_SEP = "://";
9586

96-
private ProgressMode progressMode = ProgressMode.OLDEST_ACTIONS;
9787
private int sampleSize = 3;
9888

9989
private String status;
@@ -416,9 +406,8 @@ synchronized String describe() {
416406
this(clock, 0);
417407
}
418408

419-
/** Set the progress bar mode and sample size. */
420-
void setProgressMode(ProgressMode progressMode, int sampleSize) {
421-
this.progressMode = progressMode;
409+
/** Set the progress bar sample size. */
410+
void setProgressSampleSize(int sampleSize) {
422411
this.sampleSize = Math.max(1, sampleSize);
423412
}
424413

@@ -906,29 +895,7 @@ private String countActions() {
906895
}
907896

908897
private void printActionState(AnsiTerminalWriter terminalWriter) throws IOException {
909-
switch (progressMode) {
910-
case OLDEST_ACTIONS:
911-
sampleOldestActions(terminalWriter);
912-
break;
913-
case MNEMONIC_HISTOGRAM:
914-
showMnemonicHistogram(terminalWriter);
915-
break;
916-
}
917-
}
918-
919-
private void showMnemonicHistogram(AnsiTerminalWriter terminalWriter) throws IOException {
920-
Multiset<String> mnemonicHistogram = HashMultiset.create();
921-
for (Map.Entry<Artifact, ActionState> action : activeActions.entrySet()) {
922-
mnemonicHistogram.add(action.getValue().action.getMnemonic());
923-
}
924-
List<Multiset.Entry<String>> sorted =
925-
mnemonicHistogram.entrySet().stream()
926-
.collect(
927-
Comparators.greatest(
928-
sampleSize, Comparator.comparingLong(Multiset.Entry::getCount)));
929-
for (Multiset.Entry<String> entry : sorted) {
930-
terminalWriter.newline().append(" " + entry.getElement() + " " + entry.getCount());
931-
}
898+
sampleOldestActions(terminalWriter);
932899
}
933900

934901
private void sampleOldestActions(AnsiTerminalWriter terminalWriter) throws IOException {

src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
5252
import com.google.devtools.build.lib.cmdline.Label;
5353
import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
54-
import com.google.devtools.build.lib.runtime.UiStateTracker.ProgressMode;
5554
import com.google.devtools.build.lib.runtime.UiStateTracker.StrategyIds;
5655
import com.google.devtools.build.lib.skyframe.LoadingPhaseStartedEvent;
5756
import com.google.devtools.build.lib.skyframe.PackageProgressReceiver;
@@ -334,7 +333,7 @@ public void testSampleSize() throws IOException {
334333

335334
// For various sample sizes verify the progress bar
336335
for (int i = 1; i < 11; i++) {
337-
stateTracker.setProgressMode(ProgressMode.OLDEST_ACTIONS, i);
336+
stateTracker.setProgressSampleSize(i);
338337
LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true);
339338
stateTracker.writeProgressBar(terminalWriter);
340339
String output = terminalWriter.getTranscript();
@@ -1355,43 +1354,6 @@ public void testTotalFetchesReported() throws IOException {
13551354
assertThat(output, containsString("30 fetches"));
13561355
}
13571356

1358-
private Action mockActionWithMnemonic(String mnemonic, String primaryOutput) {
1359-
Path path = outputBase.getRelative(PathFragment.create(primaryOutput));
1360-
Artifact artifact =
1361-
ActionsTestUtil.createArtifact(ArtifactRoot.asSourceRoot(Root.fromPath(outputBase)), path);
1362-
Action action = mock(Action.class);
1363-
when(action.getMnemonic()).thenReturn(mnemonic);
1364-
when(action.getPrimaryOutput()).thenReturn(artifact);
1365-
return action;
1366-
}
1367-
1368-
@Test
1369-
public void testMnemonicHistogram() throws IOException {
1370-
// Verify that the number of actions shown in the progress bar can be set as sample size.
1371-
ManualClock clock = new ManualClock();
1372-
clock.advanceMillis(Duration.ofSeconds(123).toMillis());
1373-
UiStateTracker stateTracker = new UiStateTracker(clock);
1374-
clock.advanceMillis(Duration.ofSeconds(2).toMillis());
1375-
1376-
// Start actions with 10 different mnemonics Mnemonic0-9, n+1 of each mnemonic.
1377-
for (int i = 0; i < 10; i++) {
1378-
clock.advanceMillis(Duration.ofSeconds(1).toMillis());
1379-
for (int j = 0; j <= i; j++) {
1380-
Action action = mockActionWithMnemonic("Mnemonic" + i, "action-" + i + "-" + j + ".out");
1381-
stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime()));
1382-
}
1383-
}
1384-
1385-
for (int sampleSize = 1; sampleSize < 11; sampleSize++) {
1386-
stateTracker.setProgressMode(ProgressMode.MNEMONIC_HISTOGRAM, sampleSize);
1387-
LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true);
1388-
stateTracker.writeProgressBar(terminalWriter);
1389-
String output = terminalWriter.getTranscript();
1390-
assertThat(output).contains("Mnemonic" + (10 - sampleSize) + " " + (10 - sampleSize + 1));
1391-
assertThat(output).doesNotContain("Mnemonic" + (10 - sampleSize - 1));
1392-
}
1393-
}
1394-
13951357
private static class FetchEvent implements FetchProgress {
13961358
private final String id;
13971359

0 commit comments

Comments
 (0)