[PATCH stable] py3: always flush ui streams on Python 3

Manuel Jacob me at manueljacob.de
Fri Jun 5 10:38:07 UTC 2020


On 2020-06-04 14:21, Yuya Nishihara wrote:
> On Thu, 04 Jun 2020 07:54:14 +0200, Manuel Jacob wrote:
>> On 2020-06-03 22:40, Augie Fackler wrote:
>> >> On Jun 3, 2020, at 07:13, Yuya Nishihara <yuya at tcha.org> wrote:
>> >> Flushing stdout per write() sounds bad. If that's the only way to work
>> >> around Py3 io module, we'll probably need to check if the stdout is
>> >> a tty or not.
>> >
>> > Agreed. Maybe we could sniff for a \n in the output stream, but I
>> > dunno if that's likely to be a win.
>> 
>> For status output (in the general sense, not necessarily in the
>> ui.status() sense), flushing the stream would IMHO be fine: the 
>> messages
>> should be shown as fast as possible, often end with a '\n'
> 
> Yes, that should be fine.
> 
>> For "data" output (like diffs), flushing the stream can indeed be
>> wasteful: showing single lines immediately is often not as important,
>> ui.write() is often called with small data (not complete lines), and 
>> the
>> amount of output can be large. Maybe we can introduce special methods
>> for this kind of data that uses fully buffered writes? Code that uses
>> these methods would need to flush manually.
> 
> Actually there's a method for "data" output:
> 
>  - ui.write(), which is a subset of ui._write()
>  - ui.status(), etc. -> ui._write() -> ui._writenobuf()
> 
> So it's probably okay for ui._writenobuf() to flush() per call. For 
> "data"
> output, we'll also need to call flush() occasionally so e.g.
> "hg log -r 'grep(foobar)'" will emit data incrementally. It would be 
> nice
> if this flush() can be automated, but that's up to the overhead.

I missed one thing before: By default, ui uses the streams from 
procutil, which are supposed to be line-buffered or unbuffered. That 
logic was buggy and gets fixed in the new patch series I just sent. So 
the higher layers don’t need to flush. As suggested in a comment in the 
last patch, we could make the streams from procutil buffered and push 
the responsibility to flush to higher layers.

>> Of course, we should do that only if the stream is interactive. I’m
>> concerned that calling isatty() on every write is too expensive, but I
>> need to benchmark this. BTW, there’s ui._isatty() that checks the
>> ui.nontty config. This config is not checked when setting sys.stdout 
>> to
>> line-buffered on Python 2 (by the interpreter), so am I right that we
>> should not care about this config?
> 
> That wouldn't matter, but I think ui functions should respect ui.nontty
> option consistently.
> 
>> BTW, I’m not sure I understand the comment about stderr on Windows. I
>> couldn't find any further information on this topic. Is it the case
>> that, when stderr is redirected to stdout on windows, Python fails to
>> recognize that stderr is actually interactive and doesn't set line
>> buffering?
> 
> Maybe there would be weird behavior in C stdio.

Procutil claims "Windows doesn't support line buffering. I didn’t find 
an good independent source clearly indicating that this is the case, but 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/setvbuf?view=vs-2019#remarks 
suggests that it could be the case.

Luckily, on Python 3 we’re not dependent on such implementation details 
anymore.

I think the logic to flush stderr can be replaced by code in procutil 
that ensures that procutil.stderr is always unbuffered. I’ll send a 
patch once the current series is through.

>> Is there some command that calls ui.write() many times with small data
>> and doesn’t do much else, to be used as a benchmark?
> 
> hg files?



More information about the Mercurial-devel mailing list