[RFC][PATCH 3/4] Speed improvement in fetch/clone: fetch code

[Robert Collins]

On Sat, 2005-12-10 at 19:15 +0100, Goffredo Baroncelli wrote:
> This patch changes the fetch code. Before, the original code expanded every inventory and
> copied/joined the changed weave. After that it merged the single inventory to the 
> local inventory weave.
> 
> Now the code get ( via the file_involved() function ) the file_id involved, then
> copies/joins the weave changed. After that the new code merges the two inventory
> (remote and local ). The big gain is that it isn't necessary anymore to expand 
> and merge every single inventory.

Sweet. +1 on concept :).

> 
> === modified file 'bzrlib/fetch.py'
> --- bzrlib/fetch.py	
> +++ bzrlib/fetch.py	
> @@ -108,23 +108,90 @@
>              self.pb = pb
>          self.from_branch.lock_read()
>          try:
> -            self._fetch_revisions(last_revision)
> +            #self._fetch_revisions(last_revision)

please dont comment code out: Either delete the line, or dont delete the
line. We *have* a VCS to manage old texts for us.

> +            revs = self._revs_to_fetch(last_revision )
> +            # nothing to do
> +            if revs: 
> +                self._fetch_revision_texts( revs )
> +                self._fetch_weave_texts( revs )
> +                self._fetch_inventory_weave( revs )
> +                self.count_copied += len(revs)
>          finally:
>              self.from_branch.unlock()
>              self.pb.clear()
>  
> -    def _fetch_revisions(self, last_revision):
> +    def _revs_to_fetch(self, last_revision):
>          self.last_revision = self._find_last_revision(last_revision)
>          mutter('fetch up to rev {%s}', self.last_revision)
>          if (self.last_revision is not None and 
>              self.to_branch.has_revision(self.last_revision)):
>              return
>          try:
> -            revs_to_fetch = self._compare_ancestries()
> +            branch_from_revs = set(self.from_branch.get_ancestry(self.last_revision))
>          except WeaveError:
>              raise InstallFailed([self.last_revision])
> -        self._copy_revisions(revs_to_fetch)
> -        self.new_ancestry = revs_to_fetch
> +
> +        self.dest_last_rev = self.to_branch.last_revision()
> +        branch_to_revs = set(self.to_branch.get_ancestry(self.dest_last_rev))
> +
> +        return branch_from_revs.difference( branch_to_revs )

if I read this right it returns the revision ids to return:
     def _revisionids_to_fetch(self, last_revision):
might be clearer.

> ...+
> +        self.pb.clear( )
> +
> +
> +

1 blank line between methods please. 

> +    def _fetch_inventory_weave( self, revs ):
> +        self.pb.update( "inventory merge",0,1)
> +        ...


Other than that, and perhaps a little bit too much vertical white space
in the methods, its looking good.

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/20051215/345f279f/attachment.pgp