Re: [PATCH] reduce progress updates in background

[Luke Mewburn <luke@mewburn.net>]

[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]

On Mon, Apr 13, 2015 at 10:11:09AM -0400, Nicolas Pitre wrote:
  | What if you suspend the task and push it into the background? Would be 
  | nice to inhibit progress display in that case, and resume it if the task 
  | returns to the foreground.

That's what happens; the suppression only occurs if the process is
currently background.  If I start a long-running operation (such as "git
fsck"), the progress is displayed. I then suspend & background, and the
progress is suppressed.  If I resume the process in the foreground, the
progress starts to display again at the appropriate point.

In the proposed patch, the stop_progress display for a given progress
(i.e. the one that ends in ", done.") is displayed even if in the
background so that there's some indication of progress. E.g.
  Checking object directories: 100% (256/256), done.
  Checking objects: 100% (184664/184664), done.
  Checking connectivity: 184667, done.
This is the test 'if (is_foreground || done)'.

I'm not 100% happy with my choice here, and the simpler behaviour
of "suppress all background progress output" can be achieved by
removing '|| done' from those two tests.

That still doesn't suppress _all_ output whilst in the background.
In order to do that, a larger refactor of various warning methods
would be required. I would argue that's a separate orthoganal fix.


  | Also the display() function may be called quite a lot without 
  | necessarily resulting in a display output. Therefore I'd suggest adding 
  | in_progress_fd() to the if condition right before the printf() instead.

That's an easy enough change to make (although I speculate that the
testing of the foreground status is not that big a performance issue,
especially compared the the existing performance "overhead" of printing
the progress to stderr then forcing a flush :)


Should I submit a revised patch with
(1) call in_progress_fd() just before the fprintf() as requested, and
(2) suppress all display output including the "done" call.
?


regards,
Luke.

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]