Rev 4149: Prevent regression to overhead of lock_read on pack repositories. in http://people.ubuntu.com/~robertc/baz2.0/integration

[Robert Collins]

At http://people.ubuntu.com/~robertc/baz2.0/integration

------------------------------------------------------------
revno: 4149
revision-id: robertc at robertcollins.net-20090316053331-84t8k9hv0xw7kak4
parent: robertc at robertcollins.net-20090316050819-unhkpvsvu3mzaore
committer: Robert Collins <robertc at robertcollins.net>
branch nick: integration
timestamp: Mon 2009-03-16 16:33:31 +1100
message:
  Prevent regression to overhead of lock_read on pack repositories.
=== modified file 'bzrlib/repofmt/pack_repo.py'
--- a/bzrlib/repofmt/pack_repo.py	2009-03-16 05:05:52 +0000
+++ b/bzrlib/repofmt/pack_repo.py	2009-03-16 05:33:31 +0000
@@ -1529,6 +1529,10 @@
         return [[final_rev_count, final_pack_list]]
 
     def ensure_loaded(self):
+        """Ensure we have read names from disk.
+
+        :return: True if the disk names had not been previously read.
+        """
         # NB: if you see an assertion error here, its probably access against
         # an unlocked repo. Naughty.
         if not self.repo.is_locked():
@@ -1540,8 +1544,12 @@
                 name = key[0]
                 self._names[name] = self._parse_index_sizes(value)
                 self._packs_at_load.add((key, value))
+            result = True
+        else:
+            result = False
         # populate all the metadata.
         self.all_packs()
+        return result
 
     def _parse_index_sizes(self, value):
         """Parse a string of index sizes."""
@@ -1840,14 +1848,17 @@
         This should be called when we find out that something we thought was
         present is now missing. This happens when another process re-packs the
         repository, etc.
+
+        :return: True if the in-memory list of packs has been altered at all.
         """
         # The ensure_loaded call is to handle the case where the first call
         # made involving the collection was to reload_pack_names, where we 
         # don't have a view of disk contents. Its a bit of a bandaid, and
         # causes two reads of pack-names, but its a rare corner case not struck
         # with regular push/pull etc.
-        self.ensure_loaded()
-        # This is functionally similar to _save_pack_names, but we don't write
+        first_read = self.ensure_loaded()
+        if first_read:
+            return True
         # out the new value.
         disk_nodes, _, _ = self._diff_pack_names()
         self._packs_at_load = disk_nodes

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py	2009-03-06 11:26:37 +0000
+++ b/bzrlib/tests/blackbox/test_push.py	2009-03-16 05:33:31 +0000
@@ -196,13 +196,12 @@
         t.commit(allow_pointless=True, message='first commit')
         self.reset_smart_call_log()
         self.run_bzr(['push', self.get_url('to-one')], working_dir='from')
-        rpc_count = len(self.hpss_calls)
         # This figure represent the amount of work to perform this use case. It
         # is entirely ok to reduce this number if a test fails due to rpc_count
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertEqual(20, rpc_count)
+        self.assertLength(20, self.hpss_calls)
 
     def test_push_smart_stacked_streaming_acceptance(self):
         self.setup_smart_server_with_call_log()
@@ -213,13 +212,12 @@
         self.reset_smart_call_log()
         self.run_bzr(['push', '--stacked', '--stacked-on', '../parent',
             self.get_url('public')], working_dir='local')
-        rpc_count = len(self.hpss_calls)
         # This figure represent the amount of work to perform this use case. It
         # is entirely ok to reduce this number if a test fails due to rpc_count
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertEqual(56, rpc_count)
+        self.assertLength(56, self.hpss_calls)
         remote = Branch.open('public')
         self.assertEndsWith(remote.get_stacked_on_url(), '/parent')
 

=== modified file 'bzrlib/tests/blackbox/test_shared_repository.py'
--- a/bzrlib/tests/blackbox/test_shared_repository.py	2009-02-23 15:29:35 +0000
+++ b/bzrlib/tests/blackbox/test_shared_repository.py	2009-03-16 05:33:31 +0000
@@ -114,10 +114,9 @@
         # be fixed.
         self.setup_smart_server_with_call_log()
         self.run_bzr(['init-repo', self.get_url('repo')])
-        rpc_count = len(self.hpss_calls)
         # This figure represent the amount of work to perform this use case. It
         # is entirely ok to reduce this number if a test fails due to rpc_count
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertEqual(16, rpc_count)
+        self.assertLength(16, self.hpss_calls)