This is an archive of the discontinued Mercurial Phabricator instance.

ui: Improve ui.write performance when not coloring on Windows
AbandonedPublic

Authored by joerg.sonnenberger on Jan 25 2018, 1:51 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Summary

For `hg diff`, ui.write appears as super hot in the profile with up to
60% time spend in it for larger diffs. Introduce two predicates to
decide if label processing is active at all and if it is, whether the
result can still be simply batched up. If either case is true, process
all chunks of a file in one go and use `ui.write` once. This reduces
the time of `hg diff` from 3m54s to 2m26s for the test case.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

joerg.sonnenberger retitled this revision from [ui] Improve ui.write performance when not coloring on Windows to ui: Improve ui.write performance when not coloring on Windows.Jan 25 2018, 1:59 PM
joerg.sonnenberger updated this revision to Diff 5003.
mercurial/cmdutil.py
1627

The grouping is necessary to avoid computing the full diff in memory with the associated memory use. Turns out splitting it up is even slightly faster, too. The grouper function itself is pretty generic (modulo the hard-coded fillvalue), so putting it in util.py or so would be a good idea.

lothiraldan added inline comments.
mercurial/ui.py
887

Style nit, missing new line between the two methods

yuja requested changes to this revision.Feb 3 2018, 2:54 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/cmdutil.py
1593

We no longer need this write() wrapper because no labeling is required if fp is given.

1620

If we don't care labels, we can simply use patch.diff() and .diffstat() instead of patch.diffui() and .diffstatui() respectively.

1624

Please move the import line to top.

1629

grouper() could be replaced by util.chunkbuffer() over chunks of bytes.

if nolabel:
    chunksiter = patch.diff()
    util.chunkbuffer(chunksiter)
elif batchable:
    chunksiter = patch.diffui()
    util.chunkbuffer(ui.label(chunk, label) for chunk, label in chunksiter)
mercurial/ui.py
884

Perhaps we can simply make _colormode public instead of adding
these functions?

If ui is buffered, batchable state wouldn't matter as all write()s
will be combined into a string.

This revision now requires changes to proceed.Feb 3 2018, 2:54 AM
joerg.sonnenberger abandoned this revision.Feb 3 2018, 6:32 PM