Skip to content

Commit 78ba948

Browse files
ehussehuss
committed
Improve progress bar flickering.
1 parent d75d1fb commit 78ba948

File tree

3 files changed

+130
-84
lines changed

3 files changed

+130
-84
lines changed

‎src/cargo/core/compiler/job_queue.rs

Lines changed: 114 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use jobserver::{Acquired, HelperThread};
1212
use log::{debug, info, trace};
1313

1414
use crate::core::profiles::Profile;
15-
use crate::core::{PackageId, Target, TargetKind};
15+
use crate::core::{PackageId, Target, TargetKind, Verbosity};
1616
use crate::handle_error;
1717
use crate::util;
1818
use crate::util::diagnostic_server::{self, DiagnosticPrinter};
@@ -29,7 +29,7 @@ use super::{BuildContext, BuildPlan, CompileMode, Context, Kind, Unit};
2929
/// This structure is backed by the `DependencyQueue` type and manages the
3030
/// actual compilation step of each package. Packages enqueue units of work and
3131
/// then later on the entire graph is processed and compiled.
32-
pub struct JobQueue<'a> {
32+
pub struct JobQueue<'a, 'cfg> {
3333
queue: DependencyQueue<Key<'a>, Vec<(Job, Freshness)>>,
3434
tx: Sender<Message<'a>>,
3535
rx: Receiver<Message<'a>>,
@@ -39,6 +39,7 @@ pub struct JobQueue<'a> {
3939
documented: HashSet<PackageId>,
4040
counts: HashMap<PackageId, usize>,
4141
is_release: bool,
42+
progress: Progress<'cfg>,
4243
}
4344

4445
/// A helper structure for metadata about the state of a building package.
@@ -131,9 +132,10 @@ impl<'a> JobState<'a> {
131132
}
132133
}
133134

134-
impl<'a> JobQueue<'a> {
135-
pub fn new<'cfg>(bcx: &BuildContext<'a, 'cfg>) -> JobQueue<'a> {
135+
impl<'a, 'cfg> JobQueue<'a, 'cfg> {
136+
pub fn new(bcx: &BuildContext<'a, 'cfg>) -> JobQueue<'a, 'cfg> {
136137
let (tx, rx) = channel();
138+
let progress = Progress::with_style("Building", ProgressStyle::Ratio, bcx.config);
137139
JobQueue {
138140
queue: DependencyQueue::new(),
139141
tx,
@@ -144,10 +146,11 @@ impl<'a> JobQueue<'a> {
144146
documented: HashSet::new(),
145147
counts: HashMap::new(),
146148
is_release: bcx.build_config.release,
149+
progress,
147150
}
148151
}
149152

150-
pub fn enqueue<'cfg>(
153+
pub fn enqueue(
151154
&mut self,
152155
cx: &Context<'a, 'cfg>,
153156
unit: &Unit<'a>,
@@ -231,7 +234,6 @@ impl<'a> JobQueue<'a> {
231234
// successful and otherwise wait for pending work to finish if it failed
232235
// and then immediately return.
233236
let mut error = None;
234-
let mut progress = Progress::with_style("Building", ProgressStyle::Ratio, cx.bcx.config);
235237
let total = self.queue.len();
236238
loop {
237239
// Dequeue as much work as we can, learning about everything
@@ -276,76 +278,90 @@ impl<'a> JobQueue<'a> {
276278
// to the jobserver itself.
277279
tokens.truncate(self.active.len() - 1);
278280

279-
let count = total - self.queue.len();
280-
let active_names = self
281-
.active
282-
.iter()
283-
.map(Key::name_for_progress)
284-
.collect::<Vec<_>>();
285-
drop(progress.tick_now(count, total, &format!(": {}", active_names.join(", "))));
286-
let event = self.rx.recv().unwrap();
287-
progress.clear();
288-
289-
match event {
290-
Message::Run(cmd) => {
291-
cx.bcx
292-
.config
293-
.shell()
294-
.verbose(|c| c.status("Running", &cmd))?;
295-
}
296-
Message::BuildPlanMsg(module_name, cmd, filenames) => {
297-
plan.update(&module_name, &cmd, &filenames)?;
298-
}
299-
Message::Stdout(out) => {
300-
println!("{}", out);
301-
}
302-
Message::Stderr(err) => {
303-
let mut shell = cx.bcx.config.shell();
304-
shell.print_ansi(err.as_bytes())?;
305-
shell.err().write_all(b"\n")?;
306-
}
307-
Message::FixDiagnostic(msg) => {
308-
print.print(&msg)?;
309-
}
310-
Message::Finish(key, result) => {
311-
info!("end: {:?}", key);
312-
313-
// self.active.remove_item(&key); // <- switch to this when stabilized.
314-
let pos = self
315-
.active
316-
.iter()
317-
.position(|k| *k == key)
318-
.expect("an unrecorded package has finished compiling");
319-
self.active.remove(pos);
320-
if !self.active.is_empty() {
321-
assert!(!tokens.is_empty());
322-
drop(tokens.pop());
281+
// Drain all events at once to avoid displaying the progress bar
282+
// unnecessarily.
283+
let events: Vec<_> = self.rx.try_iter().collect();
284+
let events = if events.is_empty() {
285+
self.show_progress(total);
286+
vec![self.rx.recv().unwrap()]
287+
} else {
288+
events
289+
};
290+
291+
for event in events {
292+
// CAUTION: Care must be taken to clear the progress bar if a
293+
// message is to be actually displayed. Try to avoid
294+
// unnecessarily clearing it to avoid flickering.
295+
match event {
296+
Message::Run(cmd) => {
297+
if cx.bcx.config.shell().verbosity() == Verbosity::Verbose {
298+
self.progress.clear();
299+
}
300+
cx.bcx
301+
.config
302+
.shell()
303+
.verbose(|c| c.status("Running", &cmd))?;
323304
}
324-
match result {
325-
Ok(()) => self.finish(key, cx)?,
326-
Err(e) => {
327-
let msg = "The following warnings were emitted during compilation:";
328-
self.emit_warnings(Some(msg), &key, cx)?;
329-
330-
if !self.active.is_empty() {
331-
error = Some(failure::format_err!("build failed"));
332-
handle_error(&e, &mut *cx.bcx.config.shell());
333-
cx.bcx.config.shell().warn(
334-
"build failed, waiting for other \
335-
jobs to finish...",
336-
)?;
337-
} else {
338-
error = Some(e);
305+
Message::BuildPlanMsg(module_name, cmd, filenames) => {
306+
plan.update(&module_name, &cmd, &filenames)?;
307+
}
308+
Message::Stdout(out) => {
309+
self.progress.clear();
310+
println!("{}", out);
311+
}
312+
Message::Stderr(err) => {
313+
self.progress.clear();
314+
let mut shell = cx.bcx.config.shell();
315+
shell.print_ansi(err.as_bytes())?;
316+
shell.err().write_all(b"\n")?;
317+
}
318+
Message::FixDiagnostic(msg) => {
319+
self.progress.clear();
320+
print.print(&msg)?;
321+
}
322+
Message::Finish(key, result) => {
323+
info!("end: {:?}", key);
324+
325+
// self.active.remove_item(&key); // <- switch to this when stabilized.
326+
let pos = self
327+
.active
328+
.iter()
329+
.position(|k| *k == key)
330+
.expect("an unrecorded package has finished compiling");
331+
self.active.remove(pos);
332+
if !self.active.is_empty() {
333+
assert!(!tokens.is_empty());
334+
drop(tokens.pop());
335+
}
336+
match result {
337+
Ok(()) => self.finish(key, cx)?,
338+
Err(e) => {
339+
self.progress.clear();
340+
let msg = "The following warnings were emitted during compilation:";
341+
self.emit_warnings(Some(msg), &key, cx)?;
342+
343+
if !self.active.is_empty() {
344+
error = Some(failure::format_err!("build failed"));
345+
handle_error(&e, &mut *cx.bcx.config.shell());
346+
cx.bcx.config.shell().warn(
347+
"build failed, waiting for other \
348+
jobs to finish...",
349+
)?;
350+
} else {
351+
error = Some(e);
352+
}
339353
}
340354
}
341355
}
342-
}
343-
Message::Token(acquired_token) => {
344-
tokens.push(acquired_token.chain_err(|| "failed to acquire jobserver token")?);
356+
Message::Token(acquired_token) => {
357+
tokens.push(
358+
acquired_token.chain_err(|| "failed to acquire jobserver token")?,
359+
);
360+
}
345361
}
346362
}
347363
}
348-
drop(progress);
364+
self.progress.clear();
349365

350366
let build_type = if self.is_release { "release" } else { "dev" };
351367
// NOTE: This may be a bit inaccurate, since this may not display the
@@ -384,6 +400,19 @@ impl<'a> JobQueue<'a> {
384400
}
385401
}
386402

403+
fn show_progress(&mut self, total: usize) {
404+
let count = total - self.queue.len();
405+
let active_names = self
406+
.active
407+
.iter()
408+
.map(Key::name_for_progress)
409+
.collect::<Vec<_>>();
410+
drop(
411+
self.progress
412+
.tick_now(count, total, &format!(": {}", active_names.join(", "))),
413+
);
414+
}
415+
387416
/// Executes a job in the `scope` given, pushing the spawned thread's
388417
/// handled onto `threads`.
389418
fn run(
@@ -422,27 +451,28 @@ impl<'a> JobQueue<'a> {
422451
}
423452

424453
fn emit_warnings(
425-
&self,
454+
&mut self,
426455
msg: Option<&str>,
427456
key: &Key<'a>,
428457
cx: &mut Context<'_, '_>,
429458
) -> CargoResult<()> {
430459
let output = cx.build_state.outputs.lock().unwrap();
431460
let bcx = &mut cx.bcx;
432461
if let Some(output) = output.get(&(key.pkg, key.kind)) {
433-
if let Some(msg) = msg {
434-
if !output.warnings.is_empty() {
462+
if !output.warnings.is_empty() {
463+
self.progress.clear();
464+
if let Some(msg) = msg {
435465
writeln!(bcx.config.shell().err(), "{}\n", msg)?;
436466
}
437-
}
438467

439-
for warning in output.warnings.iter() {
440-
bcx.config.shell().warn(warning)?;
441-
}
468+
for warning in output.warnings.iter() {
469+
bcx.config.shell().warn(warning)?;
470+
}
442471

443-
if !output.warnings.is_empty() && msg.is_some() {
444-
// Output an empty line.
445-
writeln!(bcx.config.shell().err())?;
472+
if msg.is_some() {
473+
// Output an empty line.
474+
writeln!(bcx.config.shell().err())?;
475+
}
446476
}
447477
}
448478

@@ -491,10 +521,12 @@ impl<'a> JobQueue<'a> {
491521
// Skip Doctest
492522
if !key.mode.is_any_test() {
493523
self.documented.insert(key.pkg);
524+
self.progress.clear();
494525
config.shell().status("Documenting", key.pkg)?;
495526
}
496527
} else {
497528
self.compiled.insert(key.pkg);
529+
self.progress.clear();
498530
if key.mode.is_check() {
499531
config.shell().status("Checking", key.pkg)?;
500532
} else {
@@ -508,6 +540,9 @@ impl<'a> JobQueue<'a> {
508540
&& !(key.mode == CompileMode::Doctest && self.compiled.contains(&key.pkg))
509541
{
510542
self.compiled.insert(key.pkg);
543+
if config.shell().verbosity() == Verbosity::Verbose {
544+
self.progress.clear();
545+
}
511546
config.shell().verbose(|c| c.status("Fresh", key.pkg))?;
512547
}
513548
}

‎src/cargo/core/compiler/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ impl Executor for DefaultExecutor {
126126

127127
fn compile<'a, 'cfg: 'a>(
128128
cx: &mut Context<'a, 'cfg>,
129-
jobs: &mut JobQueue<'a>,
129+
jobs: &mut JobQueue<'a, 'cfg>,
130130
plan: &mut BuildPlan,
131131
unit: &Unit<'a>,
132132
exec: &Arc<dyn Executor>,

‎src/cargo/util/progress.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ struct State<'cfg> {
2727
name: String,
2828
done: bool,
2929
throttle: Throttle,
30+
last_line: Option<String>,
3031
}
3132

3233
struct Format {
@@ -59,6 +60,7 @@ impl<'cfg> Progress<'cfg> {
5960
name: name.to_string(),
6061
done: false,
6162
throttle: Throttle::new(),
63+
last_line: None,
6264
}),
6365
}
6466
}
@@ -185,20 +187,29 @@ impl<'cfg> State<'cfg> {
185187
if self.format.max_width < 15 {
186188
return Ok(());
187189
}
188-
self.config.shell().status_header(&self.name)?;
190+
189191
let mut line = prefix.to_string();
190192
self.format.render(&mut line, msg);
191-
192193
while line.len() < self.format.max_width - 15 {
193194
line.push(' ');
194195
}
195196

196-
write!(self.config.shell().err(), "{}\r", line)?;
197+
// Only update if the line has changed.
198+
if self.last_line.as_ref() != Some(&line) {
199+
self.config.shell().status_header(&self.name)?;
200+
write!(self.config.shell().err(), "{}\r", line)?;
201+
self.last_line = Some(line);
202+
}
203+
197204
Ok(())
198205
}
199206

200207
fn clear(&mut self) {
201-
self.config.shell().err_erase_line();
208+
// No need to clear if the progress is not currently being displayed.
209+
if self.last_line.is_some() {
210+
self.config.shell().err_erase_line();
211+
self.last_line = None;
212+
}
202213
}
203214

204215
fn try_update_max_width(&mut self) {

0 commit comments

Comments
 (0)