Merge lp:~bzr/bzr/faster-branch-notree into lp:~bzr/bzr/trunk-old

Proposed by Ian Clatworthy on 2009-05-20
Status: Merged
Approved by: Aaron Bentley on 2009-05-21
Approved revision: 4374
Merged at revision: not available
Proposed branch: lp:~bzr/bzr/faster-branch-notree
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 44 lines
To merge this branch: bzr merge lp:~bzr/bzr/faster-branch-notree
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve on 2009-05-21
Bazaar Developers 2009-05-20 Pending
Review via email: mp+6706@code.launchpad.net

This proposal supersedes a proposal from 2009-05-18.

To post a comment you must log in.
Ian Clatworthy (ian-clatworthy) wrote : Posted in a previous version of this proposal

This patch improve the performance of branch --no-tree. On the OpenOffice trunk, the time improves from 2 minutes to 0.2 seconds.

Aaron Bentley (abentley) wrote :

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

Ian Clatworthy wrote:
> Ian Clatworthy has proposed merging lp:~bzr/bzr/faster-branch-notree into lp:bzr.

I would really prefer not to fix it this way. It is coded this way
because we expect iter_references to be efficient. To make it efficient
with RevisionTrees, we just need to exit early when the underlying
repository doesn't support tree-references, as we already do with
WorkingTree4.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkoT1isACgkQ0F+nu1YWqI05bgCdELsR0goUTvR+kqGoiZsPdX0V
alkAn2Az6NsCE+SiWZIy//tgsJe87Oc2
=Z30H
-----END PGP SIGNATURE-----

Ian Clatworthy (ian-clatworthy) wrote :

> I would really prefer not to fix it this way. It is coded this way
> because we expect iter_references to be efficient. To make it efficient
> with RevisionTrees, we just need to exit early when the underlying
> repository doesn't support tree-references, as we already do with
> WorkingTree4.

Can you explain further and/or tweak the branch, time permitting? I'm not sure I see the advantage of what you're suggesting over exiting early in Tree.iter_references() and delegating supports_tree_reference() to RevisionTree as I'm doing. I don't particularly *like* how Tree.iter_references() is coded - it could use iter_just_entries() and only calculate paths when a tree reference is found instead - but I don't see the benefit in having a custom implementation of iter_references in RevisionTree. What am I missing?

John A Meinel (jameinel) wrote :

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

Ian Clatworthy wrote:
>> I would really prefer not to fix it this way. It is coded this way
>> because we expect iter_references to be efficient. To make it efficient
>> with RevisionTrees, we just need to exit early when the underlying
>> repository doesn't support tree-references, as we already do with
>> WorkingTree4.
>
> Can you explain further and/or tweak the branch, time permitting? I'm not sure I see the advantage of what you're suggesting over exiting early in Tree.iter_references() and delegating supports_tree_reference() to RevisionTree as I'm doing. I don't particularly *like* how Tree.iter_references() is coded - it could use iter_just_entries() and only calculate paths when a tree reference is found instead - but I don't see the benefit in having a custom implementation of iter_references in RevisionTree. What am I missing?

I would guess he was voting on the earlier form, which had the 'if
supports_tree_reference' as part of BzrDir.sprout().

Obviously, Aaron needs to clarify.

John
=:->

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

iEYEARECAAYFAkoUDv8ACgkQJdeBCYSNAAMKtwCgl6W81HyZVyOmPKCnZB9JybBP
xj0AoLfXuC/h0zqRxAIAYIfJ3tgRjIqC
=5CWz
-----END PGP SIGNATURE-----

Aaron Bentley (abentley) wrote :

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

My apologies, I misread it. This is fine.

 status approved

Ian Clatworthy wrote:
>> I would really prefer not to fix it this way. It is coded this way
>> because we expect iter_references to be efficient. To make it efficient
>> with RevisionTrees, we just need to exit early when the underlying
>> repository doesn't support tree-references, as we already do with
>> WorkingTree4.
>
> Can you explain further and/or tweak the branch, time permitting? I'm not sure I see the advantage of what you're suggesting over exiting early in Tree.iter_references() and delegating supports_tree_reference() to RevisionTree as I'm doing. I don't particularly *like* how Tree.iter_references() is coded - it could use iter_just_entries() and only calculate paths when a tree reference is found instead - but I don't see the benefit in having a custom implementation of iter_references in RevisionTree. What am I missing?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkoVzscACgkQ0F+nu1YWqI322wCdHl5vRzxo3pjiSsUxQxGhLEh1
UXUAnRm7t76WyLzDPqoNrB47LR8jpECv
=gteY
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-05-21 14:06:43 +0000
+++ NEWS 2009-05-21 17:35:17 +0000
@@ -18,6 +18,8 @@
18Improvements18Improvements
19************19************
2020
21* ``bzr branch --notree`` is now faster. (Ian Clatworthy)
22
21Bug Fixes23Bug Fixes
22*********24*********
2325
2426
=== modified file 'bzrlib/revisiontree.py'
--- bzrlib/revisiontree.py 2009-05-06 05:36:28 +0000
+++ bzrlib/revisiontree.py 2009-05-21 17:35:17 +0000
@@ -45,7 +45,8 @@
45 self._rules_searcher = None45 self._rules_searcher = None
4646
47 def supports_tree_reference(self):47 def supports_tree_reference(self):
48 return True48 return getattr(self._repository._format, "supports_tree_reference",
49 False)
4950
50 def get_parent_ids(self):51 def get_parent_ids(self):
51 """See Tree.get_parent_ids.52 """See Tree.get_parent_ids.
5253
=== modified file 'bzrlib/tree.py'
--- bzrlib/tree.py 2009-05-07 05:08:46 +0000
+++ bzrlib/tree.py 2009-05-21 17:35:17 +0000
@@ -202,9 +202,10 @@
202 specific_file_ids=specific_file_ids)202 specific_file_ids=specific_file_ids)
203203
204 def iter_references(self):204 def iter_references(self):
205 for path, entry in self.iter_entries_by_dir():205 if self.supports_tree_reference():
206 if entry.kind == 'tree-reference':206 for path, entry in self.iter_entries_by_dir():
207 yield path, entry.file_id207 if entry.kind == 'tree-reference':
208 yield path, entry.file_id
208209
209 def kind(self, file_id):210 def kind(self, file_id):
210 raise NotImplementedError("Tree subclass %s must implement kind"211 raise NotImplementedError("Tree subclass %s must implement kind"