[MERGE] 40% faster partial commits

[Ian Clatworthy]

Robert Collins wrote:
> This makes partial commits walk much less data, with approximately 40%
> increase in speed.

Nice. It's only partial commit but it reduces the inner tests for
non-partial commit which is highly desirable as well.

bb:tweak

> -        self.specific_files = specific_files
> +        self.specific_files = sorted(
> +            minimum_path_selection(specific_files or ()))
> +        self.specific_file_ids = None

Thinking about this some more this morning, I think self.specific_files
should be None if specific_files is None. It's a small change but I'd
like to keep to the policy Aaron has been adopting:

* None => all
* [] => nothing

> +                # as a side effect of finding the IDs. We later use the ids we
> +                # found as input to the workng tree inventory iterator, so we

Small typo in working.

> -                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.


> +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."

> +    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.

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

Ian C.