Code review comment for lp:~vila/bzr/525571-lock-bazaar-conf-files

> As a comment, without having really read the code thoroughly.
>
> How does this handle stuff like 2 branches locking concurrently
> locations.conf. I don't know how often we do it internally, though.

TestLockableConfig.test_writes_are_serialized

>
> I think lots of filesystem locks on the bazaar directory could adversely
> affect performance on Windows. IME locking isn't too expensive if you do
> it 1 or 2 times. But if you lock and unlock on every attribute that gets
> set, then it probably starts to be an issue.

The actual code is not correct, it allows concurrent writers. If higher levels do too many updates of config variables it's a problem in the higher levels, we could imagine holding the updates until the last one, but
this can't addressed here.

Correctness comes before performance, there are many things we can do to address performance but it's way out of scope for this proposal IMHO.

>
> On a Windows host:
> $ TIMEIT -s "b = Branch.open('.')" "b.lock_write(); b.unlock()"
> 10.5msec
>
> On an Ubuntu VM on the same machine:
> $ TIMEIT -s "b = Branch.open('.')" "b.lock_write(); b.unlock()"
> 1.55msec

Thanks, good data point, but still, I haven't seen code doing massive updates of config variables either...