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

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 29, 2015, 3:52 p.m.
Message ID <20150930005209.c3bad367d6b2a82e1b4d32c0@tcha.org>
Download mbox | patch
Permalink /patch/10682/
State Rejected
Headers show

Comments

Yuya Nishihara - Sept. 29, 2015, 3:52 p.m.
On Mon, 28 Sep 2015 18:17:52 -0500, Matt Mackall wrote:
> On Mon, 2015-09-28 at 00:15 +0900, Yuya Nishihara wrote:
> > On Sat, 26 Sep 2015 15:45:45 -0700, Pierre-Yves David wrote:
> > > On 09/26/2015 01:03 AM, Yuya Nishihara wrote:
> > > > On Fri, 25 Sep 2015 13:08:09 -0500, Matt Mackall wrote:
> > > >>>> diff -r b80b2ee71a08 mercurial/ui.py
> > > >>>> --- a/mercurial/ui.py	Thu Sep 24 00:34:15 2015 -0700
> > > >>>> +++ b/mercurial/ui.py	Thu Sep 24 17:45:42 2015 -0500
> > > >>>> @@ -104,6 +104,7 @@
> > > >>>>            self._ucfg = config.config() # untrusted
> > > >>>>            self._trustusers = set()
> > > >>>>            self._trustgroups = set()
> > > >>>> +        self._outcount = 0
> > > >>>>            self.callhooks = True
> > > >>>>
> > > >>>>            if src:
> > > >>>> @@ -617,6 +618,9 @@
> > > >>>>            else:
> > > >>>>                for a in args:
> > > >>>>                    self.fout.write(str(a))
> > > >>>> +            if self._outcount < 50:
> > > >>>> +                self.fout.flush()
> > > >>>> +                self._outcount += 1
> > > >>>>
> > > >>>>        def write_err(self, *args, **opts):
> > > >>>>            self._progclear()
> > > >>>>
> > > >>>> ("hg log -k sullivan -T." is a pretty good behavior test here.)
> > > >
> > > > So we want line-buffering for first 50 lines?
> > > >
> > > > I've timed it with line-buffered stdout. It's slightly slower than fully-
> > > > buffered io, but perhaps we don't care it if we use the pager.
> > > 
> > > Look slike you got your hand on the way to control the buffer setting. \o/
> > > 
> > > We should definitely goes for line (or probably even none) buffering for 
> > > the pager pipe (the same as what we do for no-pager).
> > 
> > Okay, I'll look for the best way to control the buffering. IIRC, the buffer
> > resides in FILE API of libc as of Python 2. There are several ways to change it.
> > 
> >  a) sys.stdout = fdopen(sys.stdout.fileno(), 'wb', 1)  # 1 for line buffering
> > 
> >     can't replace all FILE* objects because sys.stdout may be aliased to
> >     different names, such as ui.fout.
> 
> I couldn't get this to work for some reason when I tried a few months
> back. But if you can get it working, this is probably the cleanest
> approach.

At least, we have to replace sys.stdout and ui.fout (and more?) by new object.
That is the difficult point of this approach. There might be many references to
the FILE object of stdout, and it won't be easy to tell whether we should
replace sys.__stdout__ or not, for example.
Matt Mackall - Sept. 29, 2015, 10:11 p.m.
On Wed, 2015-09-30 at 00:52 +0900, Yuya Nishihara wrote:
> On Mon, 28 Sep 2015 18:17:52 -0500, Matt Mackall wrote:
> > On Mon, 2015-09-28 at 00:15 +0900, Yuya Nishihara wrote:
> > > On Sat, 26 Sep 2015 15:45:45 -0700, Pierre-Yves David wrote:
> > > > On 09/26/2015 01:03 AM, Yuya Nishihara wrote:
> > > > > On Fri, 25 Sep 2015 13:08:09 -0500, Matt Mackall wrote:
> > > > >>>> diff -r b80b2ee71a08 mercurial/ui.py
> > > > >>>> --- a/mercurial/ui.py	Thu Sep 24 00:34:15 2015 -0700
> > > > >>>> +++ b/mercurial/ui.py	Thu Sep 24 17:45:42 2015 -0500
> > > > >>>> @@ -104,6 +104,7 @@
> > > > >>>>            self._ucfg = config.config() # untrusted
> > > > >>>>            self._trustusers = set()
> > > > >>>>            self._trustgroups = set()
> > > > >>>> +        self._outcount = 0
> > > > >>>>            self.callhooks = True
> > > > >>>>
> > > > >>>>            if src:
> > > > >>>> @@ -617,6 +618,9 @@
> > > > >>>>            else:
> > > > >>>>                for a in args:
> > > > >>>>                    self.fout.write(str(a))
> > > > >>>> +            if self._outcount < 50:
> > > > >>>> +                self.fout.flush()
> > > > >>>> +                self._outcount += 1
> > > > >>>>
> > > > >>>>        def write_err(self, *args, **opts):
> > > > >>>>            self._progclear()
> > > > >>>>
> > > > >>>> ("hg log -k sullivan -T." is a pretty good behavior test here.)
> > > > >
> > > > > So we want line-buffering for first 50 lines?
> > > > >
> > > > > I've timed it with line-buffered stdout. It's slightly slower than fully-
> > > > > buffered io, but perhaps we don't care it if we use the pager.
> > > > 
> > > > Look slike you got your hand on the way to control the buffer setting. \o/
> > > > 
> > > > We should definitely goes for line (or probably even none) buffering for 
> > > > the pager pipe (the same as what we do for no-pager).
> > > 
> > > Okay, I'll look for the best way to control the buffering. IIRC, the buffer
> > > resides in FILE API of libc as of Python 2. There are several ways to change it.
> > > 
> > >  a) sys.stdout = fdopen(sys.stdout.fileno(), 'wb', 1)  # 1 for line buffering
> > > 
> > >     can't replace all FILE* objects because sys.stdout may be aliased to
> > >     different names, such as ui.fout.
> > 
> > I couldn't get this to work for some reason when I tried a few months
> > back. But if you can get it working, this is probably the cleanest
> > approach.
> 
> At least, we have to replace sys.stdout and ui.fout (and more?) by new object.

I think that's fine. Just about everything goes through ui anyway.
Yuya Nishihara - Sept. 30, 2015, 3:13 p.m.
On Tue, 29 Sep 2015 17:11:43 -0500, Matt Mackall wrote:
> On Wed, 2015-09-30 at 00:52 +0900, Yuya Nishihara wrote:
> > On Mon, 28 Sep 2015 18:17:52 -0500, Matt Mackall wrote:
> > > On Mon, 2015-09-28 at 00:15 +0900, Yuya Nishihara wrote:
> > > >  a) sys.stdout = fdopen(sys.stdout.fileno(), 'wb', 1)  # 1 for line buffering
> > > > 
> > > >     can't replace all FILE* objects because sys.stdout may be aliased to
> > > >     different names, such as ui.fout.
> > > 
> > > I couldn't get this to work for some reason when I tried a few months
> > > back. But if you can get it working, this is probably the cleanest
> > > approach.
> > 
> > At least, we have to replace sys.stdout and ui.fout (and more?) by new object.
> 
> I think that's fine. Just about everything goes through ui anyway.

Ok, I'll try this way. I'll check out the use of sys.__stdout__ as we sometimes
use it to test if the given file object is a stdout.

Patch

--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -66,6 +66,7 @@  from mercurial.i18n import _
 testedwith = 'internal'
 
 def _pagersubprocess(ui, p):
+    sys.stdout = ui.fout = os.fdopen(sys.stdout.fileno(), 'wb', 1)
     pager = subprocess.Popen(p, shell=True, bufsize=-1,
                              close_fds=util.closefds, stdin=subprocess.PIPE,
                              stdout=sys.stdout, stderr=sys.stderr)