D820: progress: make ETA only consider progress made in the last minute

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Wed Sep 27 13:05:59 EDT 2017


indygreg added a comment.


  Thanks for the patch - the current behavior has bothered me as well and we should improve the time estimation algorithm for reasons you've outlined.
  
  Is there a reason you went with 60s? Have you experimented with shorter values? (Obviously this can be bikeshedded later because it is a config option and easy to change. I'm just curious where magic numbers come from.)
  
  I'm also not sure why `progress.estimate` is experimental. I think we should remove that label.

INLINE COMMENTS

> progress.py:108
> +        # experimental config: progress.estimateinterval, in seconds
> +        self.estimateinterval = self.ui.configint(
> +            'progress', 'estimateinterval')

Do we not have a configfloat()? This is being used in the context of non-integer math, so IMO it should be a float.

> progress.py:265
> +            delta = pos - self.startvals[topic]
> +            newdelta = 1.0 * delta * interval / elapsed
> +            if newdelta < 0.1: # prevent too steep changes

Nit: maybe just cast something to a float so we can avoid the useless multiply by 1.0?

REPOSITORY
  rHG Mercurial

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

To: quark, #hg-reviewers, mbthomas
Cc: indygreg, mbthomas, mercurial-devel


More information about the Mercurial-devel mailing list