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

------------------------------------------------------------
revno: 4051
revision-id: [email protected]
parent: [email protected]
parent: [email protected]
committer: Canonical.com Patch Queue Manager <[email protected]>
branch nick: +trunk
timestamp: Wed 2009-02-25 22:00:24 +0000
message:
  (jam) Batch up requests for fulltexts into 5MB (compressed) requests,
        rather than 1 per file.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
  bzrlib/tests/test_knit.py      test_knit.py-20051212171302-95d4c00dd5f11f2b
    ------------------------------------------------------------
    revno: 4039.3.8
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Wed 2009-02-25 15:16:14 -0600
    message:
      NEWS entry about batching up requests.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
    ------------------------------------------------------------
    revno: 4039.3.7
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Wed 2009-02-25 15:13:22 -0600
    message:
      Some direct tests for _group_keys_for_io
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
      bzrlib/tests/test_knit.py      
test_knit.py-20051212171302-95d4c00dd5f11f2b
    ------------------------------------------------------------
    revno: 4039.3.6
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Wed 2009-02-25 14:23:04 -0600
    message:
      Turn _split_by_prefix into a classmethod, and add direct tests.
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
      bzrlib/tests/test_knit.py      
test_knit.py-20051212171302-95d4c00dd5f11f2b
    ------------------------------------------------------------
    revno: 4039.3.5
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Wed 2009-02-25 14:12:13 -0600
    message:
      Add direct tests for _get_total_build_size.
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
      bzrlib/tests/test_knit.py      
test_knit.py-20051212171302-95d4c00dd5f11f2b
    ------------------------------------------------------------
    revno: 4039.3.4
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Tue 2009-02-24 17:06:35 -0600
    message:
      Properly determine the total number of bytes needed for a given key.
      We need to evaluate the whole build chain, in order to get the final 
build info.
      The earlier code only evaluated the final node of the chain, which gave
      unrealistically low sizes.
      With this, bzr.dev get split into 2 requests, which is reasonable.
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
    ------------------------------------------------------------
    revno: 4039.3.3
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Tue 2009-02-24 16:13:03 -0600
    message:
      Add some debugging code.
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
    ------------------------------------------------------------
    revno: 4039.3.2
    revision-id: [email protected]
    parent: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Tue 2009-02-24 15:54:05 -0600
    message:
      Batch get_record_stream(fulltexts) into 5MB requests.
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
        ------------------------------------------------------------
        revno: 3966.3.1
        revision-id: [email protected]
        parent: [email protected]
        committer: John Arbash Meinel <[email protected]>
        branch nick: knit_fill
        timestamp: Wed 2009-01-28 13:08:31 -0600
        message:
          Quick attempt at re-combining the knit requests after splitting them 
by file-id.
        modified:
          bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
    ------------------------------------------------------------
    revno: 4039.3.1
    revision-id: [email protected]
    parent: [email protected]
    committer: John Arbash Meinel <[email protected]>
    branch nick: sort_knit_fetch
    timestamp: Tue 2009-02-24 14:23:07 -0600
    message:
      Group records to read by pack file and sort by offset.
      
      This assumes that sort(index_memo) gives the right ordering, but for all 
      formats so far, this holds true.
      
      This makes a *big* difference when doing 'bzr branch --stacked' or
      'bzr checkout --lightweight'.
      
      If you don't group the requests, then the next level splits them up
      into many readv() requests, because it splits them by pack/knit file.
    modified:
      bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
=== modified file 'NEWS'
--- a/NEWS      2009-02-25 14:36:59 +0000
+++ b/NEWS      2009-02-25 22:00:24 +0000
@@ -46,6 +46,11 @@
       stream if the server is version 1.13 or greater, reducing roundtrips
       significantly. (Andrew Bennetts, Robert Collins)
 
+    * Lightweight Checkouts and Stacked Branches should both be much
+      faster over remote connections. Building the working tree now
+      batches up requests into approx 5MB requests, rather than a separate
+      request for each file. (John Arbash Meinel)
+
     * Support for GSSAPI authentication when using HTTP or HTTPS. 
       (Jelmer Vernooij)
 

=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py    2009-02-23 15:29:35 +0000
+++ b/bzrlib/knit.py    2009-02-25 21:13:22 +0000
@@ -123,6 +123,7 @@
 
 DATA_SUFFIX = '.knit'
 INDEX_SUFFIX = '.kndx'
+_STREAM_MIN_BUFFER_SIZE = 5*1024*1024
 
 
 class KnitAdapter(object):
@@ -806,6 +807,35 @@
     versioned_files.writer.end()
 
 
+def _get_total_build_size(self, keys, positions):
+    """Determine the total bytes to build these keys.
+
+    (helper function because _KnitGraphIndex and _KndxIndex work the same, but
+    don't inherit from a common base.)
+
+    :param keys: Keys that we want to build
+    :param positions: dict of {key, (info, index_memo, comp_parent)} (such
+        as returned by _get_components_positions)
+    :return: Number of bytes to build those keys
+    """
+    all_build_index_memos = {}
+    build_keys = keys
+    while build_keys:
+        next_keys = set()
+        for key in build_keys:
+            # This is mostly for the 'stacked' case
+            # Where we will be getting the data from a fallback
+            if key not in positions:
+                continue
+            _, index_memo, compression_parent = positions[key]
+            all_build_index_memos[key] = index_memo
+            if compression_parent not in all_build_index_memos:
+                next_keys.add(compression_parent)
+        build_keys = next_keys
+    return sum([index_memo[2] for index_memo
+                in all_build_index_memos.itervalues()])
+
+
 class KnitVersionedFiles(VersionedFiles):
     """Storage for many versioned files using knit compression.
 
@@ -1181,6 +1211,9 @@
                 # n = next
                 records = [(key, i_m) for key, (r, i_m, n)
                                        in position_map.iteritems()]
+                # Sort by the index memo, so that we request records from the
+                # same pack file together, and in forward-sorted order
+                records.sort(key=operator.itemgetter(1))
                 raw_record_map = {}
                 for key, data in self._read_records_iter_unchecked(records):
                     (record_details, index_memo, next) = position_map[key]
@@ -1189,7 +1222,8 @@
             except errors.RetryWithNewPacks, e:
                 self._access.reload_or_raise(e)
 
-    def _split_by_prefix(self, keys):
+    @classmethod
+    def _split_by_prefix(cls, keys):
         """For the given keys, split them up based on their prefix.
 
         To keep memory pressure somewhat under control, split the
@@ -1198,16 +1232,75 @@
         This should be revisited if _get_content_maps() can ever cross
         file-id boundaries.
 
+        The keys for a given file_id are kept in the same relative order.
+        Ordering between file_ids is not, though prefix_order will return the
+        order that the key was first seen.
+
         :param keys: An iterable of key tuples
-        :return: A dict of {prefix: [key_list]}
+        :return: (split_map, prefix_order)
+            split_map       A dictionary mapping prefix => keys
+            prefix_order    The order that we saw the various prefixes
         """
         split_by_prefix = {}
+        prefix_order = []
         for key in keys:
             if len(key) == 1:
-                split_by_prefix.setdefault('', []).append(key)
-            else:
-                split_by_prefix.setdefault(key[0], []).append(key)
-        return split_by_prefix
+                prefix = ''
+            else:
+                prefix = key[0]
+
+            if prefix in split_by_prefix:
+                split_by_prefix[prefix].append(key)
+            else:
+                split_by_prefix[prefix] = [key]
+                prefix_order.append(prefix)
+        return split_by_prefix, prefix_order
+
+    def _group_keys_for_io(self, keys, non_local_keys, positions,
+                           _min_buffer_size=_STREAM_MIN_BUFFER_SIZE):
+        """For the given keys, group them into 'best-sized' requests.
+
+        The idea is to avoid making 1 request per file, but to never try to
+        unpack an entire 1.5GB source tree in a single pass. Also when
+        possible, we should try to group requests to the same pack file
+        together.
+
+        :return: list of (keys, non_local) tuples that indicate what keys
+            should be fetched next.
+        """
+        # TODO: Ideally we would group on 2 factors. We want to extract texts
+        #       from the same pack file together, and we want to extract all
+        #       the texts for a given build-chain together. Ultimately it
+        #       probably needs a better global view.
+        total_keys = len(keys)
+        prefix_split_keys, prefix_order = self._split_by_prefix(keys)
+        prefix_split_non_local_keys, _ = self._split_by_prefix(non_local_keys)
+        cur_keys = []
+        cur_non_local = set()
+        cur_size = 0
+        result = []
+        sizes = []
+        for prefix in prefix_order:
+            keys = prefix_split_keys[prefix]
+            non_local = prefix_split_non_local_keys.get(prefix, [])
+
+            this_size = self._index._get_total_build_size(keys, positions)
+            cur_size += this_size
+            cur_keys.extend(keys)
+            cur_non_local.update(non_local)
+            if cur_size > _min_buffer_size:
+                result.append((cur_keys, cur_non_local))
+                sizes.append(cur_size)
+                cur_keys = []
+                cur_non_local = set()
+                cur_size = 0
+        if cur_keys:
+            result.append((cur_keys, cur_non_local))
+            sizes.append(cur_size)
+        trace.mutter('Collapsed %d keys into %d requests w/ %d file_ids'
+                     ' w/ sizes: %s', total_keys, len(result),
+                     len(prefix_split_keys), sizes)
+        return result
 
     def get_record_stream(self, keys, ordering, include_delta_closure):
         """Get a stream of records for keys.
@@ -1334,13 +1427,11 @@
             # XXX: get_content_maps performs its own index queries; allow state
             # to be passed in.
             non_local_keys = needed_from_fallback - absent_keys
-            prefix_split_keys = self._split_by_prefix(present_keys)
-            prefix_split_non_local_keys = self._split_by_prefix(non_local_keys)
-            for prefix, keys in prefix_split_keys.iteritems():
-                non_local = prefix_split_non_local_keys.get(prefix, [])
-                non_local = set(non_local)
-                generator = _VFContentMapGenerator(self, keys, non_local,
-                    global_map)
+            for keys, non_local_keys in self._group_keys_for_io(present_keys,
+                                                                non_local_keys,
+                                                                positions):
+                generator = _VFContentMapGenerator(self, keys, non_local_keys,
+                                                   global_map)
                 for record in generator.get_record_stream():
                     yield record
         else:
@@ -2569,6 +2660,8 @@
             return index_memo[0][:-1], index_memo[1]
         return keys.sort(key=get_sort_key)
 
+    _get_total_build_size = _get_total_build_size
+
     def _split_key(self, key):
         """Split key into a prefix and suffix."""
         return key[:-1], key[-1]
@@ -2890,6 +2983,8 @@
             return positions[key][1]
         return keys.sort(key=get_index_memo)
 
+    _get_total_build_size = _get_total_build_size
+
 
 class _KnitKeyAccess(object):
     """Access to records in .knit files."""

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py 2009-02-23 15:42:47 +0000
+++ b/bzrlib/tests/test_knit.py 2009-02-25 21:13:22 +0000
@@ -1093,6 +1093,26 @@
             call[1][1].getvalue())
         self.assertEqual({'create_parent_dir': True}, call[2])
 
+    def assertTotalBuildSize(self, size, keys, positions):
+        self.assertEqual(size,
+                         knit._get_total_build_size(None, keys, positions))
+
+    def test__get_total_build_size(self):
+        positions = {
+            ('a',): (('fulltext', False), (('a',), 0, 100), None),
+            ('b',): (('line-delta', False), (('b',), 100, 21), ('a',)),
+            ('c',): (('line-delta', False), (('c',), 121, 35), ('b',)),
+            ('d',): (('line-delta', False), (('d',), 156, 12), ('b',)),
+            }
+        self.assertTotalBuildSize(100, [('a',)], positions)
+        self.assertTotalBuildSize(121, [('b',)], positions)
+        # c needs both a & b
+        self.assertTotalBuildSize(156, [('c',)], positions)
+        # we shouldn't count 'b' twice
+        self.assertTotalBuildSize(156, [('b',), ('c',)], positions)
+        self.assertTotalBuildSize(133, [('d',)], positions)
+        self.assertTotalBuildSize(168, [('c',), ('d',)], positions)
+
     def test_get_position(self):
         transport = MockTransport([
             _KndxIndex.HEADER,
@@ -1867,6 +1887,91 @@
         self.assertEqual([], self.caught_entries)
 
 
+class TestKnitVersionedFiles(KnitTests):
+
+    def assertGroupKeysForIo(self, exp_groups, keys, non_local_keys,
+                             positions, _min_buffer_size=None):
+        kvf = self.make_test_knit()
+        if _min_buffer_size is None:
+            _min_buffer_size = knit._STREAM_MIN_BUFFER_SIZE
+        self.assertEqual(exp_groups, kvf._group_keys_for_io(keys,
+                                        non_local_keys, positions,
+                                        _min_buffer_size=_min_buffer_size))
+
+    def assertSplitByPrefix(self, expected_map, expected_prefix_order,
+                            keys):
+        split, prefix_order = KnitVersionedFiles._split_by_prefix(keys)
+        self.assertEqual(expected_map, split)
+        self.assertEqual(expected_prefix_order, prefix_order)
+
+    def test__group_keys_for_io(self):
+        ft_detail = ('fulltext', False)
+        ld_detail = ('line-delta', False)
+        f_a = ('f', 'a')
+        f_b = ('f', 'b')
+        f_c = ('f', 'c')
+        g_a = ('g', 'a')
+        g_b = ('g', 'b')
+        g_c = ('g', 'c')
+        positions = {
+            f_a: (ft_detail, (f_a, 0, 100), None),
+            f_b: (ld_detail, (f_b, 100, 21), f_a),
+            f_c: (ld_detail, (f_c, 180, 15), f_b),
+            g_a: (ft_detail, (g_a, 121, 35), None),
+            g_b: (ld_detail, (g_b, 156, 12), g_a),
+            g_c: (ld_detail, (g_c, 195, 13), g_a),
+            }
+        self.assertGroupKeysForIo([([f_a], set())],
+                                  [f_a], [], positions)
+        self.assertGroupKeysForIo([([f_a], set([f_a]))],
+                                  [f_a], [f_a], positions)
+        self.assertGroupKeysForIo([([f_a, f_b], set([]))],
+                                  [f_a, f_b], [], positions)
+        self.assertGroupKeysForIo([([f_a, f_b], set([f_b]))],
+                                  [f_a, f_b], [f_b], positions)
+        self.assertGroupKeysForIo([([f_a, f_b, g_a, g_b], set())],
+                                  [f_a, g_a, f_b, g_b], [], positions)
+        self.assertGroupKeysForIo([([f_a, f_b, g_a, g_b], set())],
+                                  [f_a, g_a, f_b, g_b], [], positions,
+                                  _min_buffer_size=150)
+        self.assertGroupKeysForIo([([f_a, f_b], set()), ([g_a, g_b], set())],
+                                  [f_a, g_a, f_b, g_b], [], positions,
+                                  _min_buffer_size=100)
+        self.assertGroupKeysForIo([([f_c], set()), ([g_b], set())],
+                                  [f_c, g_b], [], positions,
+                                  _min_buffer_size=125)
+        self.assertGroupKeysForIo([([g_b, f_c], set())],
+                                  [g_b, f_c], [], positions,
+                                  _min_buffer_size=125)
+
+    def test__split_by_prefix(self):
+        self.assertSplitByPrefix({'f': [('f', 'a'), ('f', 'b')],
+                                  'g': [('g', 'b'), ('g', 'a')],
+                                 }, ['f', 'g'],
+                                 [('f', 'a'), ('g', 'b'),
+                                  ('g', 'a'), ('f', 'b')])
+
+        self.assertSplitByPrefix({'f': [('f', 'a'), ('f', 'b')],
+                                  'g': [('g', 'b'), ('g', 'a')],
+                                 }, ['f', 'g'],
+                                 [('f', 'a'), ('f', 'b'),
+                                  ('g', 'b'), ('g', 'a')])
+
+        self.assertSplitByPrefix({'f': [('f', 'a'), ('f', 'b')],
+                                  'g': [('g', 'b'), ('g', 'a')],
+                                 }, ['f', 'g'],
+                                 [('f', 'a'), ('f', 'b'),
+                                  ('g', 'b'), ('g', 'a')])
+
+        self.assertSplitByPrefix({'f': [('f', 'a'), ('f', 'b')],
+                                  'g': [('g', 'b'), ('g', 'a')],
+                                  '': [('a',), ('b',)]
+                                 }, ['f', 'g', ''],
+                                 [('f', 'a'), ('g', 'b'),
+                                  ('a',), ('b',),
+                                  ('g', 'a'), ('f', 'b')])
+
+
 class TestStacking(KnitTests):
 
     def get_basis_and_test_knit(self):


-- 
bazaar-commits mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/bazaar-commits

Reply via email to