Re: [gentoo-portage-dev] [PATCH v4] collision_protect: use dynamic report interval

[Zac Medico]

On 1/9/19 12:33 AM, Fabian Groffen wrote:
> The reporting of files remaining can look somewhat odd since the report
> interval is hardcoded to be per 1000 objects.  Adjust this interval to
> be time based.  This means that modern (fast) machines likely will never
> see the countdown messages at all.  On slow setups the message will be
> informative that there is progress, albeit rather slowly.  While at it,
> report percentage done.
>
> Output before this patch:
>
>  * checking 6158 files for package collisions
> 5158 files remaining ...
> 4158 files remaining ...
> 3158 files remaining ...
> 2158 files remaining ...
> 1158 files remaining ...
> 158 files remaining ...
>
> Possible output after this patch on a slower machine:
>
>  * checking 6158 files for package collisions
>  48% done,  3145 files remaining ...
>  96% done,   192 files remaining ...
> 100% done
>
> Signed-off-by: Fabian Groffen <grobian@g.o>
> ---
>  lib/portage/dbapi/vartree.py | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/portage/dbapi/vartree.py b/lib/portage/dbapi/vartree.py
> index 4b91caea8..df192e6fc 100644
> --- a/lib/portage/dbapi/vartree.py
> +++ b/lib/portage/dbapi/vartree.py
> @@ -35,6 +35,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
>  	'portage.util.install_mask:install_mask_dir,InstallMask',
>  	'portage.util.listdir:dircache,listdir',
>  	'portage.util.movefile:movefile',
> +	'portage.util.monotonic:monotonic',
>  	'portage.util.path:first_existing,iter_parents',
>  	'portage.util.writeable_check:get_ro_checker',
>  	'portage.util._xattr:xattr',
> @@ -3475,13 +3476,19 @@ class dblink(object):
>  			symlink_collisions = []
>  			destroot = self.settings['ROOT']
>  			totfiles = len(file_list) + len(symlink_list)
> +			tnow = monotonic()
> +			tinterv = 2  # seconds
> +			ninterv = tnow + tinterv
> +			falign = len("%d" % totfiles)
>  			showMessage(_(" %s checking %d files for package collisions\n") % \
>  				(colorize("GOOD", "*"), totfiles))
>  			for i, (f, f_type) in enumerate(chain(
>  				((f, "reg") for f in file_list),
>  				((f, "sym") for f in symlink_list))):
> -				if i % 1000 == 0 and i != 0:
> -					showMessage(_("%d files remaining ...\n") % (totfiles - i))
> +				if monotonic() > ninterv:
> +					showMessage(_("%3d%% done,  %*d files remaining ...\n") %
> +							(falign, i * 100 / totfiles, totfiles - i))
> +					ninterv = monotonic() + tinterv
>
>  				dest_path = normalize_path(
>  					os.path.join(destroot, f.lstrip(os.path.sep)))
> @@ -3570,6 +3577,8 @@ class dblink(object):
>  							break
>  					if stopmerge:
>  						collisions.append(f)
> +			if tnow + tinterv < ninterv:
> +				showMessage(_("100% done\n"))

It took me a moment to understand the calculation here. How about if we
do something like this instead:


previous = monotonic()
progress_shown = False

for f in files:
	current = monotonic()
	if current - previous > tinterv:
	    showMessage(...)
	    previous = current
	    progress_shown = True

if progress_shown:
	showMessage(_("100% done\n"))



>  			return collisions, dirs_ro, symlink_collisions, plib_collisions
>
>  	def _lstat_inode_map(self, path_iter):
>


-- 
Thanks,
Zac