D5527: progress: write ui.progress() in terms of ui.makeprogress()

[martinvonz (Martin von Zweigbergk)]

martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I think ui.makeprogress() should be the preferred interface and we
  should deprecate ui.progress(). All in-core callers already use
  ui.makeprogress(). Moving the logic to the scmutil.progress() will let
  us make further improvements.
  
  This seems to have sped up `hg perfprogress` from 1.92 s to 1.85 s,
  perhaps because we now skip the indirection of updating the progress
  bar via ui.progress().

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/scmutil.py
  mercurial/ui.py

CHANGE DETAILS

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -1691,39 +1691,11 @@
         All topics should be marked closed by setting pos to None at
         termination.
         '''
-        if getattr(self._fmsgerr, 'structured', False):
-            # channel for machine-readable output with metadata, just send
-            # raw information
-            # TODO: consider porting some useful information (e.g. estimated
-            # time) from progbar. we might want to support update delay to
-            # reduce the cost of transferring progress messages.
-            self._fmsgerr.write(None, type=b'progress', topic=topic, pos=pos,
-                                item=item, unit=unit, total=total)
-        elif self._progbar is not None:
-            self._progbar.progress(topic, pos, item=item, unit=unit,
-                                   total=total)
-
-            # Looking up progress.debug in tight loops is expensive. The value
-            # is cached on the progbar object and we can avoid the lookup in
-            # the common case where a progbar is active.
-            if pos is None or not self._progbar.debug:
-                return
-
-        # Keep this logic in sync with above.
-        if pos is None or not self.configbool('progress', 'debug'):
-            return
-
-        if unit:
-            unit = ' ' + unit
-        if item:
-            item = ' ' + item
-
-        if total:
-            pct = 100.0 * pos / total
-            self.debug('%s:%s %d/%d%s (%4.2f%%)\n'
-                     % (topic, item, pos, total, unit, pct))
+        progress = self.makeprogress(topic, unit, total)
+        if pos is not None:
+            progress.update(pos, item=item)
         else:
-            self.debug('%s:%s %d%s\n' % (topic, item, pos, unit))
+            progress.complete()
 
     def makeprogress(self, topic, unit="", total=None):
         '''exists only so low-level modules won't need to import scmutil'''
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -1439,11 +1439,46 @@
         self.update(self.pos + step, item, total)
 
     def complete(self):
-        self.ui.progress(self.topic, None)
+        self.pos = None
+        self.unit = ""
+        self.total = None
+        self._print("")
 
     def _print(self, item):
-        self.ui.progress(self.topic, self.pos, item, self.unit,
-                         self.total)
+        if getattr(self.ui._fmsgerr, 'structured', False):
+            # channel for machine-readable output with metadata, just send
+            # raw information
+            # TODO: consider porting some useful information (e.g. estimated
+            # time) from progbar. we might want to support update delay to
+            # reduce the cost of transferring progress messages.
+            self.ui._fmsgerr.write(None, type=b'progress', topic=self.topic,
+                                   pos=self.pos, item=item, unit=self.unit,
+                                   total=self.total)
+        elif self.ui._progbar is not None:
+            self.ui._progbar.progress(self.topic, self.pos, item=item,
+                                      unit=self.unit, total=self.total)
+
+            # Looking up progress.debug in tight loops is expensive. The value
+            # is cached on the progbar object and we can avoid the lookup in
+            # the common case where a progbar is active.
+            if self.pos is None or not self.ui._progbar.debug:
+                return
+
+        # Keep this logic in sync with above.
+        if self.pos is None or not self.ui.configbool('progress', 'debug'):
+            return
+
+        if self.unit:
+            unit = ' ' + self.unit
+        if item:
+            item = ' ' + item
+
+        if self.total:
+            pct = 100.0 * self.pos / self.total
+            self.ui.debug('%s:%s %d/%d%s (%4.2f%%)\n'
+                       % (self.topic, item, self.pos, self.total, unit, pct))
+        else:
+            self.ui.debug('%s:%s %d%s\n' % (self.topic, item, self.pos, unit))
 
 def gdinitconfig(ui):
     """helper function to know if a repo should be created as general delta



To: martinvonz, #hg-reviewers
Cc: mercurial-devel