[MERGE] Faster commit

[Robert Collins]

On Mon, 2007-06-04 at 15:12 +1000, Ian Clatworthy wrote:
> The attached bundle speeds up commit, one of the areas of Bazaar most in
> need of improvement for large trees. It has a noticeable impact: 50%
> quicker for small commits and up to the same for the initial commit.
> 
> In summary, the changes do the following:
> 
> 1. buffer the report of commit messages until the end

I'm still very much against this. -1

> 2. remove calls to dirstate._validate that shouldn't be in production
>    code

+2 as John says :).

> 3. remove locking around some low level workingtree_4 calls, namely
>    id2path, kind and is_executable

I think this is fine.

> The follow-up emails I received on the correctness of buffering commit
> messages at the end suggested that yes, gnome-terminal is real slow, and
> that it probably doesn't matter if and when we report what commit does
> given some competitive systems (Hg and Git) don't bother reporting it at
> all. I'd very much like to leave it at the end for the reasons outlined
> previously (https://lists.ubuntu.com/archives/bazaar/2007q2/026089.html)
> and I'm pleased we have a better UI than other tools. FWIW, it is very
> easy with this patch to toggle the behaviour by changing how a variable
> is initialised. Of course, that doesn't resolve the complaint that the
> buffering code adds complexity (even if not much of it).

I'm against this for several reasons. Firstly, I don't think its needed.
Its cruft to deal with another product which is buggy. File a bug on
that - fine. gnome-terminal should rate-limit its updates when they are
coming at 33 a second - which is ~ the rate you are creating
(50000/(25*60)) using ballpark figures. We can tolerate *some*
workarounds for buggy tools - see the 2 second hash cache threshold for
VFAT file systems, which currently affects all hash cache operations,
unfortunately.
Secondly, you say its only a little complexity, and that to a certain
extent is true. On the other hand, its complexity *in the UI* and more
python code to run during commit. Fixing the progress bar is orthogonal
to printing out the actions as we discover them. I think the progress
bar will go as future tree changes come in anyway, which would leave us
with the code and ui complexity for no reasons.
Also, and this is proof to me of the complexity, your docstring is wrong
- AIUI its not the interaction between the progress bar and the
adds/deletes etc output, its simply outputting the adds and deletes to
the screen. Try catting a file with 50K lines in it in gnome-terminal.
I'll be surprised if its not 50% of the performance hit (the other 50 is
redrawing the bar I would expect).

Finally, I think that the complexity is occuring at the wrong place,
*IF* we are going to buffer we should be implementing that in the stream
layer, not in this semantic layer.

...
> Together with John's work on status/diff and Aaron's work on
> merge/bundle/checkout/branch, I'm very keen to get this through for
> 0.17. If the reviewers can let me know ASAP what needs to be changed
> or
> dropped to get this through, I'd greatly appreciate it.

It might be an idea to put up several much smaller patches,
specifically:

 - docs
 - working tree _validate cleanups 
 - working tree lock decorator removals
 - commit changes

that would allow us to continue discussion on the contentious stuff but
let the other things that also help performance go in ASAP.

As for 0.17, I think we freeze today no? So it should be simple changes
or RC bugfixes only - mind you you are the RM so it will be your choice.
I see the _validate and lock changes as simple, not so sure about the
rest.

-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/20070604/adea1d3d/attachment-0001.pgp