fast delta generation in brisbane-core - advice/direction needed

[Robert Collins]

On Mon, 2009-03-02 at 22:19 +1000, Ian Clatworthy wrote:

> Robert/John,
> 
> Thanks for your help on IRC re this Robert.
> 
> Here's a few things found while digging and some follow-up Qs ...
> 
> Most of time goes in chk_map.iter_changes(). log -v calls this
> for each revision (with self & basis) and, IIUIC, it loads the required
> nodes for self & basis every time.

It should only load unique nodes for the inventory map, and it should
load and keep loaded the path/directory map.

>  I wonder if we want to cache loaded
> nodes so that subsequent calls (where basis is now self) don't need to
> load (some of the) nodes from disk again? At a bare minimum, caching
> the root nodes would help I think as _ensure_root() is always called
> twice per iter_changes() call.

_ensure_root is least likely to be a cache hit (it changes if anything
under it changes). Yes, caches are are a candidate to address some of th
issues; note though that it should only be loading unique pages, so
*any* loading of a page that is the same between two inventories is a
bug, or at most a candidate bug. I'd really suggest getting a detailed
picture of whats going on before slapping a cache in. We've discussed
possible things to cache, and they include the parsed nodes, unparsed
nodes, or even whole CHKMap trees. Things to think about are cache
lifetime, cache accessibility, and cache size. Should the cache be
per-repo, or global, or ...

> There are 3 variables in chk_map.iter_changes() that smell like a
> pending optimisation but they are never updated:
> 
> >         self_seen = set()
> >         basis_seen = set()
> >         excluded_keys = set()
> 
> BTW, excluded_keys *is* looked up but never added to. How were these
> intended to be updated?

I'm not sure, I think John added them.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090302/881ce1b8/attachment.pgp