Rev 4142: (andrew) Fix performance regression (many small round-trips) when in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Fri Mar 13 03:56:28 GMT 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4142
revision-id: pqm at pqm.ubuntu.com-20090313035623-vn0615cs6bd6590e
parent: pqm at pqm.ubuntu.com-20090313031626-cy59iwvd5csfcmez
parent: andrew.bennetts at canonical.com-20090313021308-e3engqnzk1yg34aj
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2009-03-13 03:56:23 +0000
message:
  (andrew) Fix performance regression (many small round-trips) when
  	pushing to a remote pack, and improve some tests.
modified:
  bzrlib/bzrdir.py               bzrdir.py-20060131065624-156dfea39c4387cb
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/bzrdir_implementations/test_bzrdir.py test_bzrdir.py-20060131065642-0ebeca5e30e30866
  bzrlib/tests/per_repository/test_repository.py test_repository.py-20060131092128-ad07f494f5c9d26c
  bzrlib/tests/per_repository_reference/__init__.py __init__.py-20080220025549-nnm2s80it1lvcwnc-2
  bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4118.1.5
    revision-id: andrew.bennetts at canonical.com-20090313021308-e3engqnzk1yg34aj
    parent: andrew.bennetts at canonical.com-20090313011808-t6cq2iix4i7g922f
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: fix-stackable-remote-repo
    timestamp: Fri 2009-03-13 12:13:08 +1000
    message:
      Fix test_remote tests.
    modified:
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4118.1.4
    revision-id: andrew.bennetts at canonical.com-20090313011808-t6cq2iix4i7g922f
    parent: andrew.bennetts at canonical.com-20090313011750-1f4d1s2tvuq20xpd
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: fix-stackable-remote-repo
    timestamp: Fri 2009-03-13 11:18:08 +1000
    message:
      Remove redundant lock_reads in test.
    modified:
      bzrlib/tests/bzrdir_implementations/test_bzrdir.py test_bzrdir.py-20060131065642-0ebeca5e30e30866
    ------------------------------------------------------------
    revno: 4118.1.3
    revision-id: andrew.bennetts at canonical.com-20090313011750-1f4d1s2tvuq20xpd
    parent: andrew.bennetts at canonical.com-20090313010442-nr05pqjqdgio3zc6
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: fix-stackable-remote-repo
    timestamp: Fri 2009-03-13 11:17:50 +1000
    message:
      Remove redundant assertion method.
    modified:
      bzrlib/tests/bzrdir_implementations/test_bzrdir.py test_bzrdir.py-20060131065642-0ebeca5e30e30866
    ------------------------------------------------------------
    revno: 4118.1.2
    revision-id: andrew.bennetts at canonical.com-20090313010442-nr05pqjqdgio3zc6
    parent: andrew.bennetts at canonical.com-20090312085004-tshz11l4qpcmnxjc
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: fix-stackable-remote-repo
    timestamp: Fri 2009-03-13 11:04:42 +1000
    message:
      Fix failing test by adding assertRepositoriesEqual.
    modified:
      bzrlib/tests/bzrdir_implementations/test_bzrdir.py test_bzrdir.py-20060131065642-0ebeca5e30e30866
    ------------------------------------------------------------
    revno: 4118.1.1
    revision-id: andrew.bennetts at canonical.com-20090312085004-tshz11l4qpcmnxjc
    parent: pqm at pqm.ubuntu.com-20090311233307-n6zmhg1y074svja6
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: fix-stackable-remote-repo
    timestamp: Thu 2009-03-12 18:50:04 +1000
    message:
      Fix performance regression (many small round-trips) when pushing to a remote pack, and tidy the tests.
    modified:
      bzrlib/bzrdir.py               bzrdir.py-20060131065624-156dfea39c4387cb
      bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
      bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
      bzrlib/tests/per_repository/test_repository.py test_repository.py-20060131092128-ad07f494f5c9d26c
      bzrlib/tests/per_repository_reference/__init__.py __init__.py-20080220025549-nnm2s80it1lvcwnc-2
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2009-03-12 07:42:57 +0000
+++ b/bzrlib/bzrdir.py	2009-03-13 03:56:23 +0000
@@ -2823,7 +2823,6 @@
                 result._custom_format = custom_format._custom_format
             else:
                 result._custom_format = custom_format
-            result.rich_root_data = custom_format.rich_root_data
         return result
 
     def get_branch_format(self):

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2009-03-11 06:50:04 +0000
+++ b/bzrlib/remote.py	2009-03-12 08:50:04 +0000
@@ -73,9 +73,9 @@
 def response_tuple_to_repo_format(response):
     """Convert a response tuple describing a repository format to a format."""
     format = RemoteRepositoryFormat()
-    format.rich_root_data = (response[0] == 'yes')
-    format.supports_tree_reference = (response[1] == 'yes')
-    format.supports_external_lookups = (response[2] == 'yes')
+    format._rich_root_data = (response[0] == 'yes')
+    format._supports_tree_reference = (response[1] == 'yes')
+    format._supports_external_lookups = (response[2] == 'yes')
     format._network_name = response[3]
     return format
 
@@ -412,6 +412,32 @@
         self._custom_format = None
         self._network_name = None
         self._creating_bzrdir = None
+        self._supports_external_lookups = None
+        self._supports_tree_reference = None
+        self._rich_root_data = None
+
+    @property
+    def rich_root_data(self):
+        if self._rich_root_data is None:
+            self._ensure_real()
+            self._rich_root_data = self._custom_format.rich_root_data
+        return self._rich_root_data
+
+    @property
+    def supports_external_lookups(self):
+        if self._supports_external_lookups is None:
+            self._ensure_real()
+            self._supports_external_lookups = \
+                self._custom_format.supports_external_lookups 
+        return self._supports_external_lookups
+
+    @property
+    def supports_tree_reference(self):
+        if self._supports_tree_reference is None:
+            self._ensure_real()
+            self._supports_tree_reference = \
+                self._custom_format.supports_tree_reference
+        return self._supports_tree_reference
 
     def _vfs_initialize(self, a_bzrdir, shared):
         """Helper for common code in initialize."""
@@ -995,12 +1021,9 @@
 
         :param repository: A repository.
         """
-        # XXX: At the moment the RemoteRepository will allow fallbacks
-        # unconditionally - however, a _real_repository will usually exist,
-        # and may raise an error if it's not accommodated by the underlying
-        # format.  Eventually we should check when opening the repository
-        # whether it's willing to allow them or not.
-        #
+        if not self._format.supports_external_lookups:
+            raise errors.UnstackableRepositoryFormat(
+                self._format.network_name(), self.base)
         # We need to accumulate additional repositories here, to pass them in
         # on various RPC's.
         #

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2009-03-12 02:45:17 +0000
+++ b/bzrlib/repository.py	2009-03-13 03:56:23 +0000
@@ -3401,6 +3401,12 @@
     def fetch(self, revision_id=None, pb=None, find_ghosts=False,
             fetch_spec=None):
         """See InterRepository.fetch()."""
+        if self.target._client._medium._is_remote_before((1, 13)):
+            # The server won't support the insert_stream RPC, so just use
+            # regular InterPackRepo logic.  This avoids a bug that causes many
+            # round-trips for small append calls.
+            return InterPackRepo.fetch(self, revision_id=revision_id, pb=pb,
+                find_ghosts=find_ghosts, fetch_spec=fetch_spec)
         # Always fetch using the generic streaming fetch code, to allow
         # streaming fetching into remote servers.
         from bzrlib.fetch import RepoFetcher

=== modified file 'bzrlib/tests/bzrdir_implementations/test_bzrdir.py'
--- a/bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2009-03-05 12:49:55 +0000
+++ b/bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2009-03-13 01:18:08 +0000
@@ -126,21 +126,14 @@
                 for rev_id in left_repo.all_revision_ids():
                     self.assertEqual(left_repo.get_revision(rev_id),
                         right_repo.get_revision(rev_id))
-                # inventories
-                left_inv_weave = left_repo.inventories
-                right_inv_weave = right_repo.inventories
-                self.assertEqual(set(left_inv_weave.keys()),
-                    set(right_inv_weave.keys()))
-                # XXX: currently this does not handle indirectly referenced
-                # inventories (e.g. where the inventory is a delta basis for
-                # one that is fully present but that the revid for that
-                # inventory is not yet present.)
-                self.assertEqual(set(left_inv_weave.keys()),
-                    set(left_repo.revisions.keys()))
-                left_trees = left_repo.revision_trees(all_revs)
-                right_trees = right_repo.revision_trees(all_revs)
-                for left_tree, right_tree in izip(left_trees, right_trees):
-                    self.assertEqual(left_tree.inventory, right_tree.inventory)
+                # Assert the revision trees (and thus the inventories) are equal
+                sort_key = lambda rev_tree: rev_tree.get_revision_id()
+                rev_trees_a = sorted(
+                    left_repo.revision_trees(all_revs), key=sort_key)
+                rev_trees_b = sorted(
+                    right_repo.revision_trees(all_revs), key=sort_key)
+                for tree_a, tree_b in zip(rev_trees_a, rev_trees_b):
+                    self.assertEqual([], list(tree_a.iter_changes(tree_b)))
                 # texts
                 text_index = left_repo._generate_text_key_index()
                 self.assertEqual(text_index,
@@ -886,6 +879,8 @@
         dir = source.bzrdir
         target = dir.sprout(self.get_url('target'))
         self.assertNotEqual(dir.transport.base, target.transport.base)
+        target_repo = target.open_repository()
+        self.assertRepositoryHasSameItems(source.repository, target_repo)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     [
                                      './.bzr/basis-inventory-cache',
@@ -896,7 +891,7 @@
                                      './.bzr/checkout/stat-cache',
                                      './.bzr/inventory',
                                      './.bzr/parent',
-                                     './.bzr/repository/inventory.knit',
+                                     './.bzr/repository',
                                      './.bzr/stat-cache',
                                      './foo',
                                      ])

=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- a/bzrlib/tests/per_repository/test_repository.py	2009-02-27 01:02:40 +0000
+++ b/bzrlib/tests/per_repository/test_repository.py	2009-03-12 08:50:04 +0000
@@ -14,7 +14,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-"""Tests for bzrdir implementations - tests a bzrdir format."""
+"""Tests for repository implementations - tests a repository format."""
 
 from cStringIO import StringIO
 import re
@@ -808,6 +808,26 @@
         local_repo = local_bzrdir.open_repository()
         self.assertEqual(remote_backing_repo._format, local_repo._format)
 
+    # XXX: this helper probably belongs on TestCaseWithTransport
+    def make_smart_server(self, path):
+        smart_server = server.SmartTCPServer_for_testing()
+        smart_server.setUp(self.get_server())
+        remote_transport = get_transport(smart_server.get_url()).clone(path)
+        self.addCleanup(smart_server.tearDown)
+        return remote_transport
+
+    def test_clone_to_hpss(self):
+        pre_metadir_formats = [RepositoryFormat5(), RepositoryFormat6()]
+        if self.repository_format in pre_metadir_formats:
+            raise TestNotApplicable(
+                "Cannot lock pre_metadir_formats remotely.")
+        remote_transport = self.make_smart_server('remote')
+        local_branch = self.make_branch('local')
+        remote_branch = local_branch.create_clone_on_transport(remote_transport)
+        self.assertEqual(
+            local_branch.repository._format.supports_external_lookups,
+            remote_branch.repository._format.supports_external_lookups)
+
     def test_clone_unstackable_branch_preserves_stackable_repo_format(self):
         """Cloning an unstackable branch format to a somewhere with a default
         stack-on branch preserves the repository format.  (i.e. if the source

=== modified file 'bzrlib/tests/per_repository_reference/__init__.py'
--- a/bzrlib/tests/per_repository_reference/__init__.py	2009-03-11 06:50:16 +0000
+++ b/bzrlib/tests/per_repository_reference/__init__.py	2009-03-12 08:50:04 +0000
@@ -26,7 +26,9 @@
     repository,
     remote,
     )
+from bzrlib.branch import BzrBranchFormat7
 from bzrlib.bzrdir import BzrDir
+from bzrlib.repofmt.pack_repo import RepositoryFormatKnitPack6
 from bzrlib.tests import multiply_tests
 from bzrlib.tests.per_repository import (
     all_repository_format_scenarios,
@@ -69,11 +71,19 @@
     """
     result = []
     for test_name, scenario_info in all_repository_format_scenarios():
-        # For remote repositories, we need at least one external reference
-        # capable format to test it: defer this until landing such a format.
-        # if isinstance(format, remote.RemoteRepositoryFormat):
-        #     scenario[1]['bzrdir_format'].repository_format =
-        if scenario_info['repository_format'].supports_external_lookups:
+        format = scenario_info['repository_format']
+        if isinstance(format, remote.RemoteRepositoryFormat):
+            # This is a RemoteRepositoryFormat scenario.  Force the scenario to
+            # use real branch and repository formats that support references.
+            scenario_info = dict(scenario_info)
+            format = remote.RemoteRepositoryFormat()
+            format._custom_format = RepositoryFormatKnitPack6()
+            scenario_info['repository_format'] = format
+            bzrdir_format = remote.RemoteBzrDirFormat()
+            bzrdir_format.repository_format = format
+            bzrdir_format.set_branch_format(BzrBranchFormat7())
+            scenario_info['bzrdir_format'] = bzrdir_format
+        if format.supports_external_lookups:
             result.append((test_name, scenario_info))
     return result
 

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2009-03-11 06:50:04 +0000
+++ b/bzrlib/tests/test_remote.py	2009-03-13 02:13:08 +0000
@@ -51,6 +51,11 @@
 from bzrlib.smart import server, medium
 from bzrlib.smart.client import _SmartClient
 from bzrlib.symbol_versioning import one_four
+from bzrlib.tests import (
+    condition_isinstance,
+    split_suite_by_condition,
+    multiply_tests,
+    )
 from bzrlib.transport import get_transport, http
 from bzrlib.transport.memory import MemoryTransport
 from bzrlib.transport.remote import (
@@ -59,11 +64,20 @@
     RemoteTCPTransport,
 )
 
+def load_tests(standard_tests, module, loader):
+    to_adapt, result = split_suite_by_condition(
+        standard_tests, condition_isinstance(BasicRemoteObjectTests))
+    smart_server_version_scenarios = [
+        ('HPSS-v2', 
+            {'transport_server': server.SmartTCPServer_for_testing_v2_only}),
+        ('HPSS-v3', 
+            {'transport_server': server.SmartTCPServer_for_testing})]
+    return multiply_tests(to_adapt, smart_server_version_scenarios, result)
+
 
 class BasicRemoteObjectTests(tests.TestCaseWithTransport):
 
     def setUp(self):
-        self.transport_server = server.SmartTCPServer_for_testing
         super(BasicRemoteObjectTests, self).setUp()
         self.transport = self.get_transport()
         # make a branch that can be opened over the smart transport
@@ -123,6 +137,19 @@
         b = BzrDir.open_from_transport(t.clone('stackable')).open_branch()
         self.assertTrue(b._format.supports_stacking())
 
+    def test_remote_repo_format_supports_external_references(self):
+        t = self.transport
+        bd = self.make_bzrdir('unstackable', format='pack-0.92')
+        r = bd.create_repository()
+        self.assertFalse(r._format.supports_external_lookups)
+        r = BzrDir.open_from_transport(t.clone('unstackable')).open_repository()
+        self.assertFalse(r._format.supports_external_lookups)
+        bd = self.make_bzrdir('stackable', format='1.9')
+        r = bd.create_repository()
+        self.assertTrue(r._format.supports_external_lookups)
+        r = BzrDir.open_from_transport(t.clone('stackable')).open_repository()
+        self.assertTrue(r._format.supports_external_lookups)
+
 
 class FakeProtocol(object):
     """Lookalike SmartClientRequestProtocolOne allowing body reading tests."""
@@ -876,7 +903,9 @@
             'success', ('ok', vfs_url))
         bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
             _client=client)
-        branch = RemoteBranch(bzrdir, RemoteRepository(bzrdir, None),
+        repo_fmt = remote.RemoteRepositoryFormat()
+        repo_fmt._custom_format = stacked_branch.repository._format
+        branch = RemoteBranch(bzrdir, RemoteRepository(bzrdir, repo_fmt),
             _client=client)
         result = branch.get_stacked_on_url()
         self.assertEqual(vfs_url, result)
@@ -893,7 +922,7 @@
             'success', ('branch', branch_network_name))
         client.add_expected_call(
             'BzrDir.find_repositoryV3', ('stacked/',),
-            'success', ('ok', '', 'no', 'no', 'no',
+            'success', ('ok', '', 'no', 'no', 'yes',
                 stacked_branch.repository._format.network_name()))
         # called twice, once from constructor and then again by us
         client.add_expected_call(
@@ -929,7 +958,7 @@
             'success', ('branch', branch_network_name))
         client.add_expected_call(
             'BzrDir.find_repositoryV3', ('stacked/',),
-            'success', ('ok', '', 'no', 'no', 'no', network_name))
+            'success', ('ok', '', 'no', 'no', 'yes', network_name))
         # called twice, once from constructor and then again by us
         client.add_expected_call(
             'Branch.get_stacked_on_url', ('stacked/',),




More information about the bazaar-commits mailing list