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 <y...@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?
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to