[Yum-devel] [PATCH][RFC] Optimizing SqliteSack

[James Antill]

On Fri, 2007-12-14 at 11:52 +0100, Florian Festi wrote:
> 
> +class YumAvailablePackageSqlite(YumAvailablePackage):
> +
> +    prco_cache = { 'obsoletes': SmallLRUCache(maxsize=10),
> +                   'conflicts': SmallLRUCache(maxsize=10),
> +                   'requires': SmallLRUCache(maxsize=10),
> +                   'provides': SmallLRUCache(maxsize=10) }
> +

 We don't need to make prco_cache part of the API do we?

 Where did the number 10 come from? That seems like a small number of
packages to have prco info. for.
 Also it seems weird to have the LRU cache over each of prco as though
they are different. It also seems like if we are trying to save memory
(esp. for large transactions), then the obvious change is to have
packages share prco data (Eg. zlib requires is a subset of glib
requires, and I bet that's very common).

>      def __init__(self, repo, db_obj):
>          self._checksums = []
> -        self.prco = { 'obsoletes': (),
> -                      'conflicts': (),
> -                      'requires': (),
> -                      'provides': () }
> +        self.prco = SqlitePrcoDict(self)
>          self._files = {}
>          self.sack = repo.sack
>          self.repoid = repo.id
> @@ -199,20 +222,19 @@ class
> YumAvailablePackageSqlite(YumAvailablePackage, PackageObject,
> RpmBase):
>          return map(lambda x: x['fname'], cur)
>  
>      @catchSqliteException
> -    def returnPrco(self, prcotype, printable=False):
> -        if isinstance(self.prco[prcotype], tuple):

 Why are you removing part of the API?

> +    def _getPrco(self, prcotype):
> +        result = self.prco_cache[prcotype].get(self)
> +        if result is None:
>              cache = self.sack.primarydb[self.repo]
>              cur = cache.cursor()
>              query = "select name, version, release, epoch, flags from
> %s "\
>                          "where pkgKey = '%s'" % (prcotype,
> self.pkgKey)
>              executeSQL(cur, query)
> -            self.prco[prcotype] = [ ]
> -            for ob in cur:
> -                self.prco[prcotype].append((ob['name'], ob['flags'],
> -                                           (ob['epoch'],
> ob['version'], 
> -                                            ob['release'])))
> -
> -        return RpmBase.returnPrco(self, prcotype, printable)
> +            result = [ (ob['name'], ob['flags'],
> +                        (ob['epoch'], ob['version'],
> +                         ob['release'])) for ob in cur ]
> +            self.prco_cache[prcotype][self] = result

 I think this just changes the RpmBase.returnPrco() call[1], to the
prco_cache set ... it would have been nice not to move to the list
comprehension for the post to the list, IMO.

[1] I'm not 100% sure what this does, but that printable argument looks
pretty suspicious.

> +        return result

-- 
James Antill <james.antill at redhat.com>
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.baseurl.org/pipermail/yum-devel/attachments/20071214/644d02fc/attachment.pgp