[MERGE] 40% faster partial commits

Robert Collins robertc at robertcollins.net
Fri Sep 21 03:16:02 BST 2007


On Fri, 2007-09-21 at 12:00 +1000, Ian Clatworthy wrote:
> > -                ie = new_ie.copy()
> > -                ie.revision = None
> > +                if old_ie.kind == 'directory':
> > +                    self._next_progress_entry()
> > +                ie = old_ie.copy()
> 
> This is the only bit I'm not 100% sure of. Not setting the revision to
> None means that the existing entry will be left exactly as is in the
> new
> inventory. That sounds right to me. The old code though explicitly
> went
> through the trouble of calculating the parents again (and I'm cautious
> just in case it was doing that for some corner case). I can't think of
> a
> reason why it needed to and you confirmed on IRC that it didn't have
> to.

So, for sanity, I guess it should be
if not single_parent_commit:
    ie.revision = None # allow merge-case checking

I think currently we prohibit partial-tree commits after merges, which
is why no tests fail for the change I made.

> 
> > +def minimum_path_selection(paths):
> > +    """Return the smallset subset of paths which are outside paths.
> > +
> > +    :param: A container of paths.
> 
> I'd prefer "param paths: A container (non-None) of paths."

Sounds good, though the (non-None) is redundant - None is not a path.

> > +    for path in paths:
> > +        other_paths = paths.difference(set([path]))
> > +        if not is_inside_any(other_paths, path):
> > +            # this is a top level path, we must check it.
> > +            search_paths.add(path)
> > +    return search_paths
> 
> As mentioned in an earlier comment, difference doesn't need the set as
> the arg is any iterable. As you explained, this code was taken
> straight
> from another module than was highly profiled. I used timeit just now
> to
> check and leaving out the set is slightly faster. It's very minor.

Cool - thanks.

> If you're good with the above and want me to tweak and merge, let me
> know.

Please do! I hope to be able to do this myself shortly, but I'd love to
get this in.

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070921/71ee3aa3/attachment.pgp 


More information about the bazaar mailing list