Patchwork D2022: ui: improve ui.write performance when not coloring on Windows

login
register
mail settings
Submitter phabricator
Date Feb. 3, 2018, 11:32 p.m.
Message ID <differential-rev-PHID-DREV-zt3nf6dyvuv2vnc42dqm-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/27271/
State Superseded
Headers show

Comments

phabricator - Feb. 3, 2018, 11:32 p.m.
joerg.sonnenberger created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2022

AFFECTED FILES
  mercurial/logcmdutil.py
  mercurial/ui.py

CHANGE DETAILS




To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 4, 2018, 11:02 a.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> logcmdutil.py:97
> +            oldchunks = chunks
> +            chunks = patch.difflabel(lambda **kwargs: oldchunks, opts=diffopts)
> +        if ui.canbatchlabelwrites():

I slightly prefer passing `chunks` as an argument in place of `oldchunks` trick. But this is really minor nit.

If we make all diffui() batchable (e.g. _exportsingle()) as a follow up, difflabel() can
just take `chunks` as an argument.

> ui.py:881
>  
> +    def writenolabels(self, **opts):
> +        '''check if write actually uses the label'''

Can you remove unused `opts` parameter?

I don't think it will be any useful since we want to feed chunks
at once where opts may vary.

> ui.py:886
> +                return True
> +        return self._colormode is None
> +

Perhaps "label -> true" would be preferable than double negative "no label -> false".

> ui.py:890
> +        '''check if write calls with labels are batchable'''
> +        assert not self.writenolabels()
> +        # Windows color printing is special, see ``write``.

I think this assert is irrelevant since "no label" writes can be batched.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2022

To: joerg.sonnenberger, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Feb. 7, 2018, 1:24 p.m.
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.


  Dropped `**opts` and queued, thanks.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2022

To: joerg.sonnenberger, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -878,6 +878,19 @@ 
 
         return "".join(self._buffers.pop())
 
+    def writenolabels(self, **opts):
+        '''check if write actually uses the label'''
+        if self._buffers and not opts.get(r'prompt', False):
+            if not self._bufferapplylabels:
+                return True
+        return self._colormode is None
+
+    def canbatchlabelwrites(self, **opts):
+        '''check if write calls with labels are batchable'''
+        assert not self.writenolabels()
+        # Windows color printing is special, see ``write``.
+        return self._colormode != 'win32'
+
     def write(self, *args, **opts):
         '''write args to output
 
diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -79,18 +79,31 @@ 
         width = 80
         if not ui.plain():
             width = ui.termwidth()
-        chunks = patch.diff(repo, node1, node2, match, changes, opts=diffopts,
-                            prefix=prefix, relroot=relroot,
-                            hunksfilterfn=hunksfilterfn)
-        for chunk, label in patch.diffstatui(util.iterlines(chunks),
-                                             width=width):
-            write(chunk, label=label)
+
+    chunks = patch.diff(repo, node1, node2, match, changes, opts=diffopts,
+                        prefix=prefix, relroot=relroot,
+                        hunksfilterfn=hunksfilterfn)
+
+    if fp is not None or ui.writenolabels():
+        if stat:
+            chunks = patch.diffstat(util.iterlines(chunks), width=width)
+        for chunk in util.filechunkiter(util.chunkbuffer(chunks)):
+            write(chunk)
     else:
-        for chunk, label in patch.diffui(repo, node1, node2, match,
-                                         changes, opts=diffopts, prefix=prefix,
-                                         relroot=relroot,
-                                         hunksfilterfn=hunksfilterfn):
-            write(chunk, label=label)
+        if stat:
+            chunks = patch.diffstatui(util.iterlines(chunks), width=width)
+        else:
+            oldchunks = chunks
+            chunks = patch.difflabel(lambda **kwargs: oldchunks, opts=diffopts)
+        if ui.canbatchlabelwrites():
+            def gen():
+                for chunk, label in chunks:
+                    yield ui.label(chunk, label=label)
+            for chunk in util.filechunkiter(util.chunkbuffer(gen())):
+                write(chunk)
+        else:
+            for chunk, label in chunks:
+                write(chunk, label=label)
 
     if listsubrepos:
         ctx1 = repo[node1]