[MERGE][0.15] Speed up 'bzr status' in xml trees

[John Arbash Meinel]

There have been some small regressions in 'bzr status' performance for
XML trees.

This patch addresses part of that.

Basically, the new WorkingTree code will always refresh self._inventory
when you take a out a lock on a working tree. (on the first real lock,
not inbetween).

This is good from a correctness perspective, but there were a couple
side effects.

Specifically, in WorkingTree.__init__() we were calling
self.read_working_inventory().

read_working_inventory is declared as @needs_read_lock. So that call was
grabbing a read lock, which would read the inventory, and then it would
again read the inventory as part of 'read_working_inventory'.

But because this was being called in WorkingTree.__init__(), there was
no chance that we would retain this read lock. So then the next call
that actually acquires a lock on the tree, would *again* read the inventory.

So the attached patch changes:

1) WorkingTree.__init__() not default to reading the inventory.
2) WorkingTree2 overrides to always read the inventory. I think it has
to do with how BzrDir.initialize was working. But WorkingTree2 is an old
format, so I didn't feel like digging deep to figure out how to get
everything to work. All I really know is that there is a
'lock_tree_write()' call, with no available inventory to be read, and
then shortly thereafter something expects WT._inventory to be valid.
3) WorkingTree.path2id() requires a read lock. WT4 already requires this
step, and this is just updating WT3 to also have this requirement.

This is a correctness improvement, but it will cause some small
performance issues in parts of our code. For example "bzr log filename"
grabs a tree, and then uses "tree.path2id()" to get its fileid. However,
it has not locked the tree yet. (gannotate suffers a similar problem).

The real goal (IMO) would be to make Tree.lock* only reread its
inventory/dirstate itself if it knows that it is out of date. (Use a
stat fingerprint & checksum on the state file).

I don't think it is really worth it for WT3, since we are migrating away
from there. But it has been in the plans for WT4.


The test suite passes, and even passes when you make WT3 the default
format. (The only one that fails is test_info, because it checks the
exact output, and that depends on the default format).



Oh, and when did we change 'basis-inventory-cache' to use format 7
instead of format 6? It sort of throws off all of my 'bzr status'
timing. Because now the WT has changed... It doesn't completely nullify
them, because all of my tests are run with a new tree, so the basis is
simply a gunzip away (no deltas).

Though it does mean that I should re-run all of my 'bzr status' timing
tests.


This is the timing results for 'bzr status' in a bzr.dev tree.
	with basis-7	basis-6
bzr.dev	452		467
bzr14	384		363
new	389		406	

So the effect of my patch is about 60ms reduction in the time it takes
to 'bzr status' in an XML tree. It is still a small regression (~26ms),
but it bothers me a lot less than the old code (~100ms).


I am pleased to see that a cache miss is less expensive in the newer
code (15-17ms versus 21ms). Which means that our knit extraction code
got faster. But it also means my performance testing is off for all of
the older bzr implementations.

Does this have a chance for 0.15?

John
=:->


PS> I'm still getting failures for blackbox.test_merge_directive because
I don't have an SMTP host running on the local machine. Did we only fix
the non-blackbox test?
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: make_status_faster.patch
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20070322/5c061f43/attachment-0001.diff