Agreed, this LGTM. Yuya? I know you've been good at spotting issues in this area that I miss.
> On Jun 9, 2020, at 07:21, Pierre-Yves David <pierre-yves.da...@ens-lyon.org> > wrote: > > Sigh, this seems like a good enough fix for now. We can rewrite the UI layer > in rust later ;-) > > On 6/7/20 12:55 AM, Manuel Jacob wrote: >> # HG changeset patch >> # User Manuel Jacob <m...@manueljacob.de> >> # Date 1591333053 -7200 >> # Fri Jun 05 06:57:33 2020 +0200 >> # Branch stable >> # Node ID d1754bf05591d2aee0f5f9dec185faf912a21190 >> # Parent 1f114c797961733ea03d0de88390f9555a7edef4 >> # EXP-Topic fix_block-buffered_stdout >> py3: stop using block-buffered stdout to TTY on Python 3 >> The previous code (added in 227ba1afcb65) incorrectly assumed that >> sys.stdout.buffer is line-buffered. However the interpreter initializes it >> with a block-buffered stream or an unbuffered stream (when the -u option or >> the PYTHONUNBUFFERED environment variable is set), never with a line-buffered >> stream. >> To see the difference with the hg command, you need to do something slow, >> e.g. >> `hg pull https://www.mercurial-scm.org/repo/hg`. On Python 3 on Linux, >> without >> this patch, all output is printed right before the command ends. With this >> patch, output is printed almost immediately. >> I did some quick and dirty benchmarking, printing on my terminal. >> Unfavorable case: >> * for 100 times: >> * for 1000 times: >> * call `ui.write(b'x')` >> * call `ui.write(b'\n')` >> * ui.flush() >> Results: >> CPython 2.7 is unaffected (line-buffered): wall 0.231795 comb 0.230000 user >> 0.230000 sys 0.000000 >> CPython 3.8 before patch (block-buffered): wall 0.190882 comb 0.190000 user >> 0.190000 sys 0.000000 >> CPython 3.8 after patch (unbuffered): wall 0.654466 comb 0.650000 user >> 0.480000 sys 0.170000 >> CPython 3.8 hand-rolled line-buffered [*]: wall 0.218919 comb 0.220000 user >> 0.220000 sys 0.000000 >> Favorable case: >> * for 100000 times: >> * call `ui.write(b'Testing write performance\n')` >> Results: >> CPython 2.7 is unaffected (line-buffered): wall 0.717060 comb 0.710000 user >> 0.510000 sys 0.200000 >> CPython 3.8 before patch (block-buffered): wall 0.255527 comb 0.260000 user >> 0.180000 sys 0.080000 >> CPython 3.8 after patch (unbuffered): wall 0.616500 comb 0.620000 user >> 0.450000 sys 0.170000 >> CPython 3.8 hand-rolled line-buffered [*]: wall 0.811805 comb 0.810000 user >> 0.570000 sys 0.240000 >> [*] For this, I implemented a simplified version of what is suggested in the >> TODO: stdout was set to block-buffered and ui.write() flushed the stream if >> the output contains a newline character. >> diff -r 1f114c797961 -r d1754bf05591 mercurial/utils/procutil.py >> --- a/mercurial/utils/procutil.py Thu Apr 30 00:33:00 2020 -0400 >> +++ b/mercurial/utils/procutil.py Fri Jun 05 06:57:33 2020 +0200 >> @@ -55,12 +55,20 @@ >> # on Windows). >> # Python 3 rolls its own standard I/O streams. >> if isatty(stdout): >> + # TODO: Consider mitigating the performance impact of using unbuffered >> + # writes on Python 3 and Windows, e.g. by using fully buffered streams >> and >> + # ensuring on a higher level that lines are flushed. >> if pycompat.iswindows: >> # Windows doesn't support line buffering >> stdout = os.fdopen(stdout.fileno(), 'wb', 0) >> - elif not pycompat.ispy3: >> - # on Python 3, stdout (sys.stdout.buffer) is already line buffered >> and >> - # buffering=1 is not handled in binary mode >> + elif pycompat.ispy3: >> + # Python 3 doesn't support line buffering on binary streams. Use the >> + # underlying unbuffered raw stream. If the ``raw`` attribute doesn't >> + # exist, stdout is already unbuffered or replaced by a custom >> stream. >> + # TODO: After Mercurial 5.4, use this code path on Python 3 on >> Windows. >> + # We shouldn't risk changing working behavior in the stable branch. >> + stdout = getattr(stdout, 'raw', stdout) >> + else: >> stdout = os.fdopen(stdout.fileno(), 'wb', 1) >> if pycompat.iswindows: >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > -- > Pierre-Yves David > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
