Re: [gentoo-portage-dev] [PATCH] portdbapi.cp_list: cache repo associations (bug 650814)

[Brian Dolbec]

On Sat, 14 Jul 2018 21:05:39 -0700
Zac Medico <zmedico@g.o> wrote:

> Make portdbapi.cp_list return _pkg_str instances that have a 'repo'
> attribute (bindbapi.cp_list already does this), with results
> in ascending order by (pkg.version, repo.priority). Optimize
> portdbapi.findname2 to use the 'repo' attribute to enable cached
> results for files previously found by the portdbapi.cp_list
> method, avoiding filesystem access when possible. Optimize the
> depgraph._iter_match_pkgs_atom method by elimination of the repo
> loop, since portdbapi.cp_list now returns separate items when the
> same package version is found in multiple repos.
>
> Bug: https://bugs.gentoo.org/650814
> ---
>  pym/_emerge/depgraph.py       | 12 +++---------
>  pym/portage/dbapi/porttree.py | 41
> +++++++++++++++++++++++++++++------------ 2 files changed, 32
> insertions(+), 21 deletions(-)
>
> diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
> index 42857c1a5..b63d4f242 100644
> --- a/pym/_emerge/depgraph.py
> +++ b/pym/_emerge/depgraph.py
> @@ -5613,10 +5613,6 @@ class depgraph(object):
>  		if cp_list:
>  			atom_set =
> InternalPackageSet(initial_atoms=(atom,), allow_repo=True)
> -			if atom.repo is None and hasattr(db,
> "getRepositories"):
> -				repo_list =
> db.getRepositories(catpkg=atom_exp.cp)
> -			else:
> -				repo_list = [atom.repo]
>
>  			# descending order
>  			cp_list.reverse()
> @@ -5624,13 +5620,11 @@ class depgraph(object):
>  				# Call match_from_list on one cpv at
> a time, in order # to avoid unnecessary match_from_list comparisons on
>  				# versions that are never yielded
> from this method.
> -				if not match_from_list(atom_exp,
> [cpv]):
> -					continue
> -				for repo in repo_list:
> -
> +				if match_from_list(atom_exp, [cpv]):
>  					try:
>  						pkg = self._pkg(cpv,
> pkg_type, root_config,
> -
> installed=installed, onlydeps=onlydeps, myrepo=repo)
> +
> installed=installed, onlydeps=onlydeps,
> +
> myrepo=getattr(cpv, 'repo', None)) except
> portage.exception.PackageNotFound: pass
>  					else:
> diff --git a/pym/portage/dbapi/porttree.py
> b/pym/portage/dbapi/porttree.py index 3e36024ff..f6076ee2b 100644
> --- a/pym/portage/dbapi/porttree.py
> +++ b/pym/portage/dbapi/porttree.py
> @@ -459,6 +459,9 @@ class portdbapi(dbapi):
>  			mytree = self.treemap.get(myrepo)
>  			if mytree is None:
>  				return (None, 0)
> +		elif mytree is not None:
> +			# myrepo enables cached results when
> available
> +			myrepo =
> self.repositories.location_map.get(mytree)
>  		mysplit = mycpv.split("/")
>  		psplit = pkgsplit(mysplit[1])
> @@ -495,6 +498,14 @@ class portdbapi(dbapi):
>  		relative_path = mysplit[0] + _os.sep + psplit[0] +
> _os.sep + \ mysplit[1] + ".ebuild"
>
> +		# There is no need to access the filesystem when the
> package
> +		# comes from this db and the package repo attribute
> corresponds
> +		# to the desired repo, since the file was previously
> found by
> +		# the cp_list method.
> +		if (myrepo is not None and myrepo == getattr(mycpv,
> 'repo', None)
> +			and self is getattr(mycpv, '_db', None)):
> +			return (mytree + _os.sep + relative_path,
> mytree) +
>  		for x in mytrees:
>  			filename = x + _os.sep + relative_path
>  			if _os.access(_unicode_encode(filename,
> @@ -950,18 +961,23 @@ class portdbapi(dbapi):
>  				return cachelist[:]
>  		mysplit = mycp.split("/")
>  		invalid_category = mysplit[0] not in self._categories
> -		d={}
> +		# Process repos in ascending order by repo.priority,
> so that
> +		# stable sort by version produces results ordered by
> +		# (pkg.version, repo.priority).
>  		if mytree is not None:
>  			if isinstance(mytree, basestring):
> -				mytrees = [mytree]
> +				repos =
> [self.repositories.get_repo_for_location(mytree)] else:
>  				# assume it's iterable
> -				mytrees = mytree
> +				repos =
> [self.repositories.get_repo_for_location(location)
> +					for location in mytree]
>  		elif self._better_cache is None:
> -			mytrees = self.porttrees
> +			repos = list(self.repositories)
>  		else:
> -			mytrees = [repo.location for repo in
> self._better_cache[mycp]]
> -		for oroot in mytrees:
> +			repos = reversed(self._better_cache[mycp])
> +		mylist = []
> +		for repo in repos:
> +			oroot = repo.location
>  			try:
>  				file_list =
> os.listdir(os.path.join(oroot, mycp)) except OSError:
> @@ -986,16 +1002,17 @@ class portdbapi(dbapi):
>  						writemsg(_("\nInvalid
> ebuild version: %s\n") % \ os.path.join(oroot, mycp, x),
> noiselevel=-1) continue
> -
> d[_pkg_str(mysplit[0]+"/"+pf, db=self)] = None
> -		if invalid_category and d:
> +
> mylist.append(_pkg_str(mysplit[0]+"/"+pf, db=self, repo=repo.name))
> +		if invalid_category and mylist:
>  			writemsg(_("\n!!! '%s' has a category that
> is not listed in " \ "%setc/portage/categories\n") % \
>  				(mycp,
> self.settings["PORTAGE_CONFIGROOT"]), noiselevel=-1) mylist = []
> -		else:
> -			mylist = list(d)
> -		# Always sort in ascending order here since it's
> handy
> -		# and the result can be easily cached and reused.
> +		# Always sort in ascending order here since it's
> handy and
> +		# the result can be easily cached and reused. Since
> mylist
> +		# is initially in ascending order by repo.priority,
> stable
> +		# sort by version produces results in ascending
> order by
> +		# (pkg.version, repo.priority).
>  		self._cpv_sort_ascending(mylist)
>  		if self.frozen and mytree is None:
>  			cachelist = mylist[:]

looks fine

-- 
Brian Dolbec <dolsen>