Re: [gentoo-portage-dev] [PATCH] portdbapi: Add cache to avoid repeated failing os.access calls

[David James <davidjames@google.com>]

[-- Attachment #1: Type: text/plain, Size: 2512 bytes --]

On Tue, Feb 18, 2014 at 1:02 PM, Sebastian Luther <SebastianLuther@gmx.de>wrote:

> ---
>  pym/portage/dbapi/porttree.py | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/pym/portage/dbapi/porttree.py b/pym/portage/dbapi/porttree.py
> index 590e3c5..2dc406b 100644
> --- a/pym/portage/dbapi/porttree.py
> +++ b/pym/portage/dbapi/porttree.py
> @@ -254,6 +254,7 @@ class portdbapi(dbapi):
>
>                 self._aux_cache = {}
>                 self._broken_ebuilds = set()
> +               self._findname2_cache = {}
>
>         @property
>         def _event_loop(self):
> @@ -372,6 +373,14 @@ class portdbapi(dbapi):
>                 the file we wanted.
>                 If myrepo is not None it will find packages from this
> repository(overlay)
>                 """
> +               cache_key = (mycpv, mytree, myrepo)
> +               try:
> +                       return self._findname2_cache[cache_key]
> +               except KeyError:
> +                       self._findname2_cache[cache_key] = (None, 0)
>

To me, it seems potentially error-prone to cache a (potentially) incorrect
value and then correct it later. Would it be possible to refactor your
patch so that we only cache the value when we know the final answer?


> +               except TypeError:
>

In what circumstances does it happen that mytree / myrepo are unhashable
types? Can you add a comment to explain this?

+                       cache_key = None
> +
>                 if not mycpv:
>                         return (None, 0)
>
> @@ -383,6 +392,8 @@ class portdbapi(dbapi):
>                 mysplit = mycpv.split("/")
>                 psplit = pkgsplit(mysplit[1])
>                 if psplit is None or len(mysplit) != 2:
> +                       if cache_key:
> +                               del self._findname2_cache[cache_key]

                        raise InvalidPackageName(mycpv)
>
>                 # For optimal performace in this hot spot, we do manual
> unicode
> @@ -402,6 +413,8 @@ class portdbapi(dbapi):
>                         filename = x + _os.sep + relative_path
>                         if _os.access(_unicode_encode(filename,
>                                 encoding=encoding, errors=errors),
> _os.R_OK):
> +                               if cache_key:
> +                                       self._findname2_cache[cache_key] =
> (filename, x)
>                                 return (filename, x)
>                 return (None, 0)
>
> --
> 1.8.3.2
>
>
>

[-- Attachment #2: Type: text/html, Size: 3811 bytes --]