[MERGE][#230567] Faster (local) branch

Ian Clatworthy ian.clatworthy at internode.on.net
Wed May 28 05:37:36 BST 2008


John Arbash Meinel wrote:
> Ian Clatworthy wrote:
> | John Arbash Meinel wrote:

> I thought that 'needs_rename' was meant to handle that. (So that when TT is
> populating the limbo directory it knows what it could put in a semi-final
> location, and what it had to put at the top level.)

It does. But the current code path still walks every item checking it and
setting executability if required. By splitting the logic into two small
loops as I've done, we can put the final tree in place much faster.

> If the change you made is just adding the '.bzr/checkout/conflicts'
> file, then
> it is probably just representative of a different code path. (One would set
> no_conflicts=True, I imagine.)
> 
> I'm not worried about having an empty control file there.

Me neither. My concern is why it's appearing now when it didn't before.
I'll dig deeper and confirm that the file is indeed empty. If it's not,
I've broken things somehow.

> Don't you need to set the flags on the Inventory entries? Certainly
> apply_inventory_delta would need to know them. (Otherwise on Win32, the
> dirstate
> won't have the 'this should have the exec bit' set, and a 'bzr co && bzr
> st'
> would show property modifications for all the executable files.

I don't think I do. The inventory entries for the target tree ought to
match those in the source tree - they don't need to be reconstructed again.

> +    # If there are existing files outside of .bzr, we need to use a
> +    # safer but slower algorithm. (This only happens when checkout
> +    # is run in a directory with existing files.)
> +    existing_files = False
> +    for num, _unused in enumerate(wt.list_files()):
> +        if num > 0:
> +            existing_files = True
> +            break
> 
> I thought I wouldn't like this, but it seems that list_files() is lazy
> about
> descending into directories, so it should end up in a single
> os.listdir() call.

I made sure that was the case before using this API.

> Oh, we should also be careful about a few other things...
> 
> 1) By default we do a 'status' in the source branch (if it has a working
> tree)
> and then copy files from there that are unmodified. This saves us from
> unpacking
> them from the repository. This probably has a large influence on some
> benchmarks.  Especially if you
> 
> 2) do/don't have a source working tree or
> 3) have/haven't run bzr status to have hot cache and/or populated
> dirstate cache.

That's right. The patch is independent of these factors though, i.e.
it will speed things up regardless. I suspect there's scope for improving
things further by looking at (3) in particular.

Ian C.



More information about the bazaar mailing list