[MERGE] only lock branches once in "bzr missing"

[Martin Pool]

2009/1/12 Benjamin Peterson <benjamin at python.org>:
> On Sun, Jan 11, 2009 at 8:44 PM, Martin Pool <mbp at canonical.com> wrote:
>> Hi,
>>
>> Thanks for the patch.
>>
>> This seems functionally correct, but I'm wondering why you wanted to add
>> it.  Taking another lock on an already-locked object should just check
>> and increment a counter, so it's pretty cheap.
>
> Ah, I didn't know if you considered it bad practice to try to lock and
> unlock a branch multiple times. Feel free to reject it if that's not
> the case.

I'd put it like this, though others may care to tweak or rephrase it:
the most important thing (taking correctness as implicit) is not to do
unnecessary physical IO to lock/unlock; after that we want the code to
be clear and not to have too much accounting overhead.

In this case, I would say the clarity of having the cli-level code use
a public interface is a bit more important than having it bump and
release two lock counters.  If it was inside an inner loop it would be
different.  There are probably several other code paths that
implicitly lock when it's reasonable to expect the caller to do it and
they could be replaced by a faster version that does not.

If you're interested in looking in to this running with -Dlock or
log+file:///.... may help you find cases where we do unnecessary
locking.

-- 
Martin <http://launchpad.net/~mbp/>