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

[Zac Medico]

On 07/14/2018 09:42 PM, Brian Dolbec wrote:
> 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

Thanks, merged:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=27eeeb6e4fc8e68efa003b2db5bbd8cc4afe336a

-- 
Thanks,
Zac