Merge lp:~jameinel/bzr/2.4-pull-iter-changes-780677 into lp:bzr

[John A Meinel]

Update WT.pull to use fast Repository iter_changes when .fast_deltas is true. (bug #780677)

This is another small patch to improve the performance of "bzr pull". It is one of the cases where using DirStateRevisionTree is worse than RevisionTree because 2a format repos can compute iter_charges much faster.

This patch by itself drops "bzr pull" from about 29s down to about 17s on my machine. However, when coupled with https://code.launchpad.net/~jameinel/bzr/2.4-set-parent-trees-delta-282941/+merge/60541 it drops "bzr pull" time down to 3.7s. (I'll update the numbers in "WhatsNew" once both of them land.)

For bzr.dev on my machine, the change is less pronounced, but it is still ~1.5s down to 1.2s.

Next on the block is probably 'bzr update'. Though WT._update_tree is pretty ugly because of all the various merges that can occur.

[Andrew Bennetts (spiv)]

Nice!

I wonder if we should push this down into WorkingTree.basis_tree() rather than just special-casing in pull()? Or perhaps this could happen in an InterTree optimiser? Anyway, that's something we can worry about later while we enjoy this improvement landing on trunk :)

Thanks for updating the table in whats-new too.

review: Approve

[John A Meinel (jameinel)]

So there are times where WT.basis_tree() is faster, namely WT.iter_changes(DirStateRevisionTree). Unfortunately RevisionTree.iter_changes(DirStateRevisionTree) is slower.

However, I think we might have the logic that WT.iter_changes(RevisionTree) will notice RT.get_revision_id() is in self.get_parent_ids() and use the fast path. So we might just get rid of DirStateRevisionTree entirely for the cases where WT.branch.repository._format.fast_deltas is true. Something to look into.

[John A Meinel (jameinel)]

sent to pqm by email

[John A Meinel (jameinel)]

sent to pqm by email

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 5/11/2011 5:13 AM, Andrew Bennetts wrote:
> Review: Approve
> Nice!
>
> I wonder if we should push this down into WorkingTree.basis_tree() rather than just special-casing in pull()? Or perhaps this could happen in an InterTree optimiser? Anyway, that's something we can worry about later while we enjoy this improvement landing on trunk :)

I looked into that. It certainly isn't hard to do. ATM, though, I'm not
100% sure of the ramifications. Any code that uses Tree.inventory is
going to be a lot faster using DirStateRevisionTree, rather than
RevisionTree. (or iter_entries_by_dir, etc).

I'd really have to audit the code to make sure that basis_tree is only
being used for iter_changes.

Of course, the other option is to create another InterTree optimizer.
One that looks between DirStateRevisionTree and RevisionTree and just
grabs another RevisionTree and runs iter_changes on the difference.

So for now, I'm sticking with this, but I'm willing to revisit it in the
future.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3KkCsACgkQJdeBCYSNAAOGagCgjmyZHlnmXFH07G/JXgAS33TW
fqoAoICKi9BDA3KBbNxi5lOJ1RftXiqz
=6PCK
-----END PGP SIGNATURE-----

=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py	2011-05-10 09:30:33 +0000
+++ bzrlib/workingtree.py	2011-05-11 11:48:38 +0000
@@ -1032,6 +1032,11 @@
             new_revision_info = self.branch.last_revision_info()
             if new_revision_info != old_revision_info:
                 repository = self.branch.repository
+                if repository._format.fast_deltas:
+                    parent_ids = self.get_parent_ids()
+                    if parent_ids:
+                        basis_id = parent_ids[0]
+                        basis_tree = repository.revision_tree(basis_id)
                 basis_tree.lock_read()
                 try:
                     new_basis_tree = self.branch.basis_tree()

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2011-05-10 09:34:35 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2011-05-11 11:48:38 +0000
@@ -30,6 +30,14 @@
   more efficiently.  For a simple branch it reduces the number of
   round-trips by about 20%.  (Andrew Bennetts)
 
+* ``bzr pull`` now properly triggers the fast
+  ``CHKInventory.iter_changes`` rather than the slow generic
+  inter-Inventory changes. It used to use a ``DirStateRevisionTree`` as
+  one of the source trees, which is faster when we have to read the whole
+  inventory anyway, but much slower when we can get just the delta out of
+  the repository. On a 70k record tree, this changes ``bzr pull`` from 28s
+  down to 17s. (John Arbash Meinel, #780677)
+
 * Slightly reduced memory consumption when fetching into a 2a repository
   by reusing existing caching a little better.  (Andrew Bennetts)
 

=== modified file 'doc/en/whats-new/whats-new-in-2.4.txt'
--- doc/en/whats-new/whats-new-in-2.4.txt	2011-05-10 09:34:35 +0000
+++ doc/en/whats-new/whats-new-in-2.4.txt	2011-05-11 11:48:38 +0000
@@ -63,11 +63,12 @@
 trees. A possibly incomplete list is as follows for running commands on a
 70k file tree::
 
-    bzr-2.3 bzr-2.4 action
-    3m39s   1m03s   bzr co --lightweight
-      38s      6s   bzr revert
-    4m47s     27s   bzr merge
-    4m58s     32s   bzr up
+    bzr-2.3.1 bzr-2.3.2 bzr-2.4 action
+    3m39s                 1m03s   bzr co --lightweight
+      38s                    6s   bzr revert
+    4m47s                   27s   bzr merge
+    4m45s           29s     17s   bzr pull
+    4m58s                   32s   bzr up
     
 
 Faster stacked branches