[MERGE] Update VersionedFileStore for the changes to VersionedFile

[Robert Collins]

On Wed, 2008-04-09 at 11:00 +0300, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> | This removes a bunch of transaction stuff from VersionedFileStore now
> | that VersionedFiles that are around detect their validity automatically.
> |
> | -Rob
> |
> 
> BB:tweak
> 
> +    * ``VersionedFileStore`` no longer uses the transaction parameter given
> +      to most methods; amongst other things this means that the
> +      get_weave_or_empty method no longer guarantees errors on a missing weave
> +      in a readonly transaction, and no longer caches versioned file instances
> +      which reduces memory pressure (but requires more careful management by
> +      callers to preserve performance.  (Robert Collins)
> 
> ^- You need a closing ')' on your (but...
> 
> I'm also a little concerned about the caveat. Because we traditionally haven't
> worried about it, I'm concerned that we'll have a subtle performance regression
> here that might bite us.
> 
> Especially for stuff like 'bzr log' which would repeatedly call into the
> Repository to "get_revisions()". It specifically *should* do that, because it
> wants to read the first 10, then the next 50, then the next ... etc. That allows
> the first display to be fast.
> 
> I'm very happy to not have to use the transaction parameters, as I always found
> them to be clumsy. (repo.do_something(foo, repo.get_transaction())). However, I
> think we have been slowly deprecating Store as a real concept, preferring to
> have the api on Repo instead.

Note that packs already bypass the transaction map; we don't cache file
texts at all anywhere for packs; packs cache the entire index as it gets
parsed rather than different versioned files in an adhoc manner.

> And you have this note specifically:
> ~                     self._generate_root_texts(revs)
> +                    # NB: This currently reopens the inventory weave in source;
> +                    # using a full get_data_stream instead would avoid this.
> ~                     self._fetch_inventory_weave(revs, pb)
> 
> I suppose if this is only penalizing Knits then it is fairly ok. Though I know
> *I* would notice a difference until I finish my multi-convert plugin to finally
> upgrade my main public repo. (It is on a slow machine, so I want to pull packs
> from other locations, but I really don't want to have to pay the full upgrade +
> reconcile on that machine.)
> 
> Otherwise I'm happy with the patch.

It goes from 1 read to 2 with this patch (as the log inspecting http
test notes). Once I finish my fetch work it will go back down to 1
again. For knit users. A format we are agreed we should nag people to
upgrade from.

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- 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 : https://lists.ubuntu.com/archives/bazaar/attachments/20080410/09940e15/attachment-0001.pgp