Bazaar Fast Import

Merge lp://staging/~jameinel/bzr-fastimport/fastimport-kg into lp://staging/bzr-fastimport

  1. fastimport-kg
  2. Merge into trunk
Proposed by John A Meinel
Status: Merged
Approved by: Ian Clatworthy
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://staging/~jameinel/bzr-fastimport/fastimport-kg
Merge into: lp://staging/bzr-fastimport
Prerequisite: lp://staging/~jameinel/bzr-fastimport/less-sticky
Diff against target: 71 lines (+38/-2)
1 file modified
revision_store.py (+38/-2)
To merge this branch: bzr merge lp://staging/~jameinel/bzr-fastimport/fastimport-kg
Reviewer Review Type Date Requested Status
Ian Clatworthy Approve
Review via email: mp+15459@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

This is also dependent on
https://code.edge.launchpad.net/~jameinel/bzr/2.1.0b4-kg-add-node/+merge/15457

However the code should be compatible either way (by using a getattr() check).

Anyway, this replaces the graph heads functionality of the commit builder by a KnownGraph implementation, which should be a lot faster. Though it only comes into play when we have an actual merge ancestry, which isn't a lot of xserver (most of it is linear).

The main downside is that it seems to actually generate a different number of file texts than the old code. Which surprises me enough that I think it should be looked at.

lp://staging/~jameinel/bzr-fastimport/fastimport-kg updated
273. By John A Meinel

Some debugging code. It looks like the main bugs involve files that are deleted and restored.

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

So just to clarify, do you think this is safe to land? Are you suggesting that it might be fixing a bug, introducing one or not sure yet? :-)

review: Needs Information
Revision history for this message
John A Meinel (jameinel) wrote :

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

Ian Clatworthy wrote:
> Review: Needs Information
> So just to clarify, do you think this is safe to land? Are you suggesting that it might be fixing a bug, introducing one or not sure yet? :-)

I think it is safe to land, and probably fixes some bugs based on graph
convergence stuff.

I think this plus some of the follow up patches should allow a bit of
performance recovery, but ideally I would profile with something like FF
to make sure I know where the bottlenecks are.

John
=:->

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

iEYEARECAAYFAksd+AgACgkQJdeBCYSNAAP+dgCeNgx2/tZpH+rfqpon81/fdbvF
JN4AoMaeOx2qkq7uid3o3ziiBvaKGycd
=DqPF
-----END PGP SIGNATURE-----

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Please merge to trunk.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'revision_store.py'
2--- revision_store.py 2009-12-01 15:18:10 +0000
3+++ revision_store.py 2009-12-01 15:18:10 +0000
4@@ -18,8 +18,16 @@
5
6 import cStringIO
7
8-from bzrlib import errors, inventory, knit, lru_cache, osutils, trace
9-from bzrlib import revision as _mod_revision
10+from bzrlib import (
11+ errors,
12+ graph as _mod_graph,
13+ inventory,
14+ knit,
15+ lru_cache,
16+ osutils,
17+ revision as _mod_revision,
18+ trace,
19+ )
20
21
22 class _TreeShim(object):
23@@ -150,6 +158,8 @@
24 :param repository: the target repository
25 """
26 self.repo = repo
27+ self._graph = None
28+ self._use_known_graph = True
29 self._supports_chks = getattr(repo._format, 'supports_chks', False)
30
31 def expects_rich_root(self):
32@@ -346,6 +356,28 @@
33 parents=rev.parent_ids, config=None, timestamp=rev.timestamp,
34 timezone=rev.timezone, committer=rev.committer,
35 revprops=rev.properties, revision_id=rev.revision_id)
36+ if self._graph is None and self._use_known_graph:
37+ if (getattr(_mod_graph, 'GraphThunkIdsToKeys', None) is None
38+ or getattr(_mod_graph.KnownGraph, 'add_node', None) is None):
39+ self._use_known_graph = False
40+ else:
41+ self._graph = self.repo.revisions.get_known_graph_ancestry(
42+ [(r,) for r in rev.parent_ids])
43+ if self._graph is not None:
44+ orig_heads = builder._heads
45+ def thunked_heads(file_id, revision_ids):
46+ # self._graph thinks in terms of keys, not ids, so translate
47+ # them
48+ # old_res = orig_heads(file_id, revision_ids)
49+ if len(revision_ids) < 2:
50+ res = set(revision_ids)
51+ else:
52+ res = set([h[0] for h in
53+ self._graph.heads([(r,) for r in revision_ids])])
54+ # if old_res != res:
55+ # import pdb; pdb.set_trace()
56+ return res
57+ builder._heads = thunked_heads
58
59 if rev.parent_ids:
60 basis_rev_id = rev.parent_ids[0]
61@@ -370,6 +402,10 @@
62 rev.inv_sha1 = builder.inv_sha1
63 builder.repository.add_revision(builder._new_revision_id, rev,
64 builder.new_inventory, builder._config)
65+ if self._graph is not None:
66+ # TODO: Use StaticTuple and .intern() for these things
67+ self._graph.add_node((builder._new_revision_id,),
68+ [(p,) for p in rev.parent_ids])
69
70 if signature is not None:
71 raise AssertionError('signatures not guaranteed yet')

Subscribers

People subscribed via source and target branches