[MERGE][1.0] New annotate merge that performs well on packs

Aaron Bentley aaron.bentley at utoronto.ca
Mon Dec 3 00:53:56 GMT 2007


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

Robert Collins wrote:
> On Sun, 2007-12-02 at 13:42 -0500, Aaron Bentley wrote:
>> So that's 1.10x as slow.
> 
> I think that's tolerable. If you know where the extra 10% is coming
> from, noting that down somewhere might be good,

It seems like an apples-and-oranges comparison.  They're very different
approaches to acquiring the same basic data.

But, I've actually managed to improve speed on knits by halving text
accesses with a simple last-accessed cache.

Old implementation (best of 3):
real    0m1.878s
user    0m1.708s
sys     0m0.136s

New implementation (best of 3):
real    0m1.823s
user    0m1.624s
sys     0m0.164s

This is now 1.03 x as fast as the old implementation.

> PlanMergeVersionedFile looks like something I would expect to find in
> either merge.py or versionedfile.py, and for PlanMerge definitely I
> would initially look in merge.py.

I think the relationships are a bit confused-- ultimately, I think tree
merging should be one module, and the text mergers should probably have
their own (maybe several).  But for now, I'm happy to be consistent with
our current practice.

> So one thing to consider - putting the
> two new classes into merge.py and possibly versionedfile.py.

Done.

> Should we deprecate VersionedFile.plan_merge?

I don't think we need to.  In fact, we could easily implement plan_merge
on _PlanMergeVersionedFile.

> Should we deprecate merge._plan_annotate_merge?

There's nothing wrong with the code if you have cheap annotations, so
we may want to resurrect it in the future.

> Lastly, you've used public class names for the new objects, if you
> intend them to be an external interface this is great

_PlanMergeVersionedFile isn't public.  PlanMerge was intended to be public.

 - and you should
> mention them in the NEWS file under INTERNALS

Done.

> as well as noting a
> reference towards them in the developer guide.

There are no existing documents to update, so I'll leave that for now.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHU1Ok0F+nu1YWqI0RAhXEAJ9zB/JxJktAKWrm2oV98+HAz1wghQCfSy/h
FcQNupAMkgRdLTx71Ia4YrA=
=I7Dv
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fast-plan-merge-2.patch
Type: text/x-diff
Size: 33496 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20071202/ae1e40c5/attachment-0001.bin 


More information about the bazaar mailing list