Patchwork [1,of,2,V2] log: flush stdout/err for the first log entries

login
register
mail settings
Submitter Durham Goode
Date Sept. 24, 2015, 7:20 p.m.
Message ID <7a9aaaadaed9887a956c.1443122432@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/10615/
State Accepted
Headers show

Comments

Durham Goode - Sept. 24, 2015, 7:20 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1443118388 25200
#      Thu Sep 24 11:13:08 2015 -0700
# Node ID 7a9aaaadaed9887a956cb6a1e79be527e6f4a20f
# Parent  a0eff7ebc2aebb32cf4c8da276104102ff37d786
log: flush stdout/err for the first log entries

There have been problems with the pager where we get no results until the python
buffer has been filled. Let's fix that by manually flushing the buffer for the
first 50 commits from log. 50 was chosen because it will likely fill the users
screen and doesn't introduce a significant overhead.

The overhead of this is negiligble. I see no perf difference when running log on
100,000 commits.
Matt Mackall - Sept. 24, 2015, 7:49 p.m.
On Thu, 2015-09-24 at 12:20 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1443118388 25200
> #      Thu Sep 24 11:13:08 2015 -0700
> # Node ID 7a9aaaadaed9887a956cb6a1e79be527e6f4a20f
> # Parent  a0eff7ebc2aebb32cf4c8da276104102ff37d786
> log: flush stdout/err for the first log entries
> 
> There have been problems with the pager where we get no results until the python
> buffer has been filled. Let's fix that by manually flushing the buffer for the
> first 50 commits from log. 50 was chosen because it will likely fill the users
> screen and doesn't introduce a significant overhead.
> 
> The overhead of this is negiligble. I see no perf difference when running log on
> 100,000 commits.

Still confused as to why this isn't in the ui code.
Durham Goode - Sept. 24, 2015, 8:02 p.m.
On 9/24/15 12:49 PM, Matt Mackall wrote:
> On Thu, 2015-09-24 at 12:20 -0700, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1443118388 25200
>> #      Thu Sep 24 11:13:08 2015 -0700
>> # Node ID 7a9aaaadaed9887a956cb6a1e79be527e6f4a20f
>> # Parent  a0eff7ebc2aebb32cf4c8da276104102ff37d786
>> log: flush stdout/err for the first log entries
>>
>> There have been problems with the pager where we get no results until the python
>> buffer has been filled. Let's fix that by manually flushing the buffer for the
>> first 50 commits from log. 50 was chosen because it will likely fill the users
>> screen and doesn't introduce a significant overhead.
>>
>> The overhead of this is negiligble. I see no perf difference when running log on
>> 100,000 commits.
> Still confused as to why this isn't in the ui code.
This is in addition to the ui code.  I have a patch that adjusts the ui 
code to flush every N seconds, but it causes some tests to fail (since 
they didn't on the timing of the progress bar output) so I sent these 
patches first.

We still need these 2 patches though.  Imagine a scenario where you run 
hg log on a directory that has only had one commit total (and it was 
recent). Immediately one commit is printed into the buffer (before the 
flush time limit is hit).  Then half an hour goes buy while we scan the 
rest of the repository for another commit in that directory.  We end up 
not showing the user that commit until the command is completely 
finished.  Printing the first N units prevents this experience.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4658,6 +4658,13 @@  def log(ui, repo, *pats, **opts):
         if displayer.flush(ctx):
             count += 1
 
+        # We want the first batch of results to show up as soon as possible, so
+        # let's flush. 50 was chosen because 50 flushes adds very little
+        # overhead, but will likely fill the user's screen even if they are
+        # printing one line per commit.
+        if count < 50:
+            ui.flush()
+
     displayer.close()
 
 @command('manifest',