| Submitter | Manuel Jacob |
|---|---|
| Date | June 22, 2020, 12:02 a.m. |
| Message ID | <1d9781c4f9bcd2df5745.1592784154@tmp> |
| Download | mbox | patch |
| Permalink | /patch/46541/ |
| State | New |
| Headers | show |
Comments
On Mon, 22 Jun 2020 02:02:34 +0200, Manuel Jacob wrote: > # HG changeset patch > # User Manuel Jacob <me@manueljacob.de> > # Date 1591314853 -7200 > # Fri Jun 05 01:54:13 2020 +0200 > # Node ID 1d9781c4f9bcd2df5745b7e9d880dbd5b46c559e > # Parent 61719b9658b156548634c216ffb2b9188266293b > # EXP-Topic perfwrite2 > perf: add another command to test performance of writing to output streams > > The command was used recently while finding a solution for a buffering bug > (eventually fixed in f9734b2d59cc (the changeset description uses a different > benchmark)). > > Calling the new command "perfwrite2" to keep the existing command "perfwrite" > unchanged is not very clever, but I didn’t have a better idea. Maybe it's okay to remove the current perfwrite command so long as it can be achieved by using the new implementation. > In comparison to the other command, the new command is much more flexible. While > using it, the focus was on testing small writes. For this reason, by default it > calls ui.write() 100 times with a single byte plus one newline byte, for 100 > lines. > > diff --git a/contrib/perf.py b/contrib/perf.py > --- a/contrib/perf.py > +++ b/contrib/perf.py > @@ -3810,6 +3810,50 @@ > fm.end() > > > +@command( > + b'perfwrite2', > + formatteropts > + + [ > + (b'', b'write-method', b'write', b'ui write method'), > + (b'', b'nlines', 100, b'number of lines'), > + (b'', b'nitems', 100, b'number of items (per line)'), > + (b'', b'item', b'x', b'item that is written'), > + (b'', b'batch-line', None, b'pass whole line to write method at once'), > + (b'', b'flush-line', None, b'flush after each line'), > + ], > +) > +def perfwrite2(ui, repo, testedtemplate=None, **opts): ^^^^^^^^^^^^^^ Nit: unused argument?
On 2020-06-22 12:58, Yuya Nishihara wrote: > On Mon, 22 Jun 2020 02:02:34 +0200, Manuel Jacob wrote: >> # HG changeset patch >> # User Manuel Jacob <me@manueljacob.de> >> # Date 1591314853 -7200 >> # Fri Jun 05 01:54:13 2020 +0200 >> # Node ID 1d9781c4f9bcd2df5745b7e9d880dbd5b46c559e >> # Parent 61719b9658b156548634c216ffb2b9188266293b >> # EXP-Topic perfwrite2 >> perf: add another command to test performance of writing to output >> streams >> >> The command was used recently while finding a solution for a buffering >> bug >> (eventually fixed in f9734b2d59cc (the changeset description uses a >> different >> benchmark)). >> >> Calling the new command "perfwrite2" to keep the existing command >> "perfwrite" >> unchanged is not very clever, but I didn’t have a better idea. > > Maybe it's okay to remove the current perfwrite command so long as it > can > be achieved by using the new implementation. It would be `perfwrite2 --nlines=100000 --nitems=1 --item='Testing write performance' --batch-line`. Do you think it would be a good idea to replace the old command (using the same name), but doing something different by default? >> In comparison to the other command, the new command is much more >> flexible. While >> using it, the focus was on testing small writes. For this reason, by >> default it >> calls ui.write() 100 times with a single byte plus one newline byte, >> for 100 >> lines. >> >> diff --git a/contrib/perf.py b/contrib/perf.py >> --- a/contrib/perf.py >> +++ b/contrib/perf.py >> @@ -3810,6 +3810,50 @@ >> fm.end() >> >> >> +@command( >> + b'perfwrite2', >> + formatteropts >> + + [ >> + (b'', b'write-method', b'write', b'ui write method'), >> + (b'', b'nlines', 100, b'number of lines'), >> + (b'', b'nitems', 100, b'number of items (per line)'), >> + (b'', b'item', b'x', b'item that is written'), >> + (b'', b'batch-line', None, b'pass whole line to write method >> at once'), >> + (b'', b'flush-line', None, b'flush after each line'), >> + ], >> +) >> +def perfwrite2(ui, repo, testedtemplate=None, **opts): > ^^^^^^^^^^^^^^ > Nit: unused argument?
On Mon, 22 Jun 2020 13:13:04 +0200, Manuel Jacob wrote: > On 2020-06-22 12:58, Yuya Nishihara wrote: > > On Mon, 22 Jun 2020 02:02:34 +0200, Manuel Jacob wrote: > >> # HG changeset patch > >> # User Manuel Jacob <me@manueljacob.de> > >> # Date 1591314853 -7200 > >> # Fri Jun 05 01:54:13 2020 +0200 > >> # Node ID 1d9781c4f9bcd2df5745b7e9d880dbd5b46c559e > >> # Parent 61719b9658b156548634c216ffb2b9188266293b > >> # EXP-Topic perfwrite2 > >> perf: add another command to test performance of writing to output > >> streams > >> > >> The command was used recently while finding a solution for a buffering > >> bug > >> (eventually fixed in f9734b2d59cc (the changeset description uses a > >> different > >> benchmark)). > >> > >> Calling the new command "perfwrite2" to keep the existing command > >> "perfwrite" > >> unchanged is not very clever, but I didn’t have a better idea. > > > > Maybe it's okay to remove the current perfwrite command so long as it > > can > > be achieved by using the new implementation. > > It would be `perfwrite2 --nlines=100000 --nitems=1 --item='Testing write > performance' --batch-line`. > > Do you think it would be a good idea to replace the old command (using > the same name), but doing something different by default? Yes. I don't think perfwrite is used as a source parameter of performance testing, and even if it were, the command argument can be easily adjusted. The change can be documented in relnotes.
Patch
diff --git a/contrib/perf.py b/contrib/perf.py --- a/contrib/perf.py +++ b/contrib/perf.py @@ -3810,6 +3810,50 @@ fm.end() +@command( + b'perfwrite2', + formatteropts + + [ + (b'', b'write-method', b'write', b'ui write method'), + (b'', b'nlines', 100, b'number of lines'), + (b'', b'nitems', 100, b'number of items (per line)'), + (b'', b'item', b'x', b'item that is written'), + (b'', b'batch-line', None, b'pass whole line to write method at once'), + (b'', b'flush-line', None, b'flush after each line'), + ], +) +def perfwrite2(ui, repo, testedtemplate=None, **opts): + """microbenchmark ui.write (and others) with small writes + """ + opts = _byteskwargs(opts) + + write = getattr(ui, _sysstr(opts[b'write_method'])) + nlines = int(opts[b'nlines']) + nitems = int(opts[b'nitems']) + item = opts[b'item'] + batch_line = opts.get(b'batch_line') + flush_line = opts.get(b'flush_line') + + if batch_line: + line = item * nitems + b'\n' + + def benchmark(): + for i in pycompat.xrange(nlines): + if batch_line: + write(line) + else: + for i in pycompat.xrange(nitems): + write(item) + write(b'\n') + if flush_line: + ui.flush() + ui.flush() + + timer, fm = gettimer(ui, opts) + timer(benchmark) + fm.end() + + def uisetup(ui): if util.safehasattr(cmdutil, b'openrevlog') and not util.safehasattr( commands, b'debugrevlogopts'