Branch.lock_read() rather expensive

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
> 
>>>What I found was that quite a bit of time was taken up, just in
>>>lock_read(). I figured part of this is because nothing takes out a read
>>>lock beforehand, so it takes out 1 read lock for each revision it parses.
>>>To prove this, I wrapped the common_ancestor() call with a lock/unlock
>>>statement, and this is what I found
> 
> 
> Ah.  I didn't understand the mechanics here.  I assumed if I didn't
> explicitly lock it, it wouldn't be locked at all.
> 
> 
>>>I'm guessing that the common_ancestor,revision_graph, and
>>>get_intervening_revisions() are too expensive as it is, but I thought I
>>>would point out a fairly major performance issue.
> 
> 
> revision_graph can be reimplemented in terms of the inventory weave, the
> way is_ancestor is.  I don't understand the weave API well enough to do
> this.

Well, having worked with the weaves, I know the private structures that
would give you want you want. (_parent contains the integer parent ids,
and lookup() can give you the number from a name _names has the name for
a given integer).

I was considering doing that as well, I just didn't know if it was
appropriate to dive into the weave privates.
Also, you have MultipleRevisionSource, but really I think you want
MultipleAncestorSource, or somesuch.

> 
> node_distances could be memoized (well if we assume there are no ghost
> revisions.  We can invalidate the cache when a ghost has been fixed.)

I don't know if people are doing changeset as a common operation. But if
merge uses it as well, then it might be worth memoizing.

John
=:->

> 
> Aaron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051015/bbe67b71/attachment.pgp