Merge lp:~jameinel/bzr/2.2-move-to-front-if-changed-562429 into lp:bzr

Proposed by John A Meinel
Status: Merged
Merged at revision: not available
Proposed branch: lp:~jameinel/bzr/2.2-move-to-front-if-changed-562429
Merge into: lp:bzr
Diff against target: 66 lines (+25/-9)
2 files modified
NEWS (+4/-0)
bzrlib/index.py (+21/-9)
To merge this branch: bzr merge lp:~jameinel/bzr/2.2-move-to-front-if-changed-562429
Reviewer Review Type Date Requested Status
Andrew Bennetts Approve
Review via email: mp+23371@code.launchpad.net

Description of the change

I haven't actually confirmed that this fixes bug #562429, but it fits the symptoms, and it does fix a problem that *I* encountered.

Basically, spiv's new 'reorder indexes' code works well, but it tries to re-order even when the indexes are already at the beginning.

This is a pretty trivial change, but in my case:
  branch.revision_history()

drops from 1.4s down to 0.62s.

To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

Makes sense, and thanks for improving my code!

I wonder if it would be faster again to teach iter_entries not to call _move_to_front at all if len(hit_indices) == num. of times it looped through 'for index in self._indices'? Probably not much.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-04-14 04:35:47 +0000
3+++ NEWS 2010-04-14 05:26:34 +0000
4@@ -67,6 +67,10 @@
5 read calls. An incremental pull via plain HTTP takes half the time and
6 bytes for a moderately large repository. (Andrew Bennetts)
7
8+* Index lookups only re-order the indexes when the hit files aren't
9+ already first. Reduces the cost of reordering
10+ (John Arbash Meinel, #562429)
11+
12 * Less code is loaded at startup. (Cold-cache start time is about 10-20%
13 less.)
14 (Martin Pool, #553017)
15
16=== modified file 'bzrlib/index.py'
17--- bzrlib/index.py 2010-04-08 07:01:10 +0000
18+++ bzrlib/index.py 2010-04-14 05:26:34 +0000
19@@ -1418,6 +1418,10 @@
20 _move_to_front propagates to all objects in self._sibling_indices by
21 calling _move_to_front_by_name.
22 """
23+ if self._indices[:len(hit_indices)] == hit_indices:
24+ # The 'hit_indices' are already at the front (and in the same
25+ # order), no need to re-order
26+ return
27 hit_names = self._move_to_front_by_index(hit_indices)
28 for sibling_idx in self._sibling_indices:
29 sibling_idx._move_to_front_by_name(hit_names)
30@@ -1431,19 +1435,27 @@
31 if 'index' in debug.debug_flags:
32 mutter('CombinedGraphIndex reordering: currently %r, promoting %r',
33 indices_info, hit_indices)
34- hit_indices_info = []
35 hit_names = []
36- unhit_indices_info = []
37- for name, idx in indices_info:
38+ unhit_names = []
39+ new_hit_indices = []
40+ unhit_indices = []
41+
42+ for offset, (name, idx) in enumerate(indices_info):
43 if idx in hit_indices:
44- info = hit_indices_info
45 hit_names.append(name)
46+ new_hit_indices.append(idx)
47+ if len(new_hit_indices) == len(hit_indices):
48+ # We've found all of the hit entries, everything else is
49+ # unhit
50+ unhit_names.extend(self._index_names[offset+1:])
51+ unhit_indices.extend(self._indices[offset+1:])
52+ break
53 else:
54- info = unhit_indices_info
55- info.append((name, idx))
56- final_info = hit_indices_info + unhit_indices_info
57- self._indices = [idx for (name, idx) in final_info]
58- self._index_names = [name for (name, idx) in final_info]
59+ unhit_names.append(name)
60+ unhit_indices.append(idx)
61+
62+ self._indices = new_hit_indices + unhit_indices
63+ self._index_names = hit_names + unhit_names
64 if 'index' in debug.debug_flags:
65 mutter('CombinedGraphIndex reordered: %r', self._indices)
66 return hit_names