[MERGE] Faster commit

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> Ian Clatworthy wrote:
> 
>> Looks like my testing was substandard - quite a few tests have indeed
>> broken since the implicit locking was removed. I'll fix this, take out
>> the message buffering (too contentious) and resubmit.
>>
>> Ian C.
>>
> 
> Tests all passing again and message buffering backed-out. In the
> interests of limiting risk, I've backed out the change where id2path()
> locking went from implicit (done for the user) to explicit (expected to
> be done by the user) - it broke too many tests as lots of calls use it
> internally. Likewise, message writing to stdout changed to stderr rather
> than change lots of commit tests.

This is actually why I was proposing to add a new function (either _kind() or
kind_no_lock(), etc). Which would provide the new behavior.

The problem is that what you would want is to change "kind" to just be a
wrapper around "kind_no_lock()" but for API compatibility, you need to have
"kind_no_lock()" default to calling "kind()" (in the case of plugins providing
trees that don't provide kind_no_lock() yet).

So for API compatibility reasons you need:

@needs_read_lock
def kind(self, file_id):
    return self._kind(file_id)

# Default implementation on Branch
def kind_no_lock(self, file_id):
    return self.kind(file_id)

# Implementation on Branch5+:
def kind_no_lock(self, file_id):
    return self._kind(file_id)


def _kind(self, file_id):
    # All the code that used to be in "kind()".


This is the difficulty based on our statement of compatibility. Because you
have bi-directional api compatibility. You have callers who are currently
calling "kind()" without grabbing a lock, and you have plugins like bzr-svn
which provide a WT implementation, and it should be compatible with other
versions of bzr.

I'm okay with relaxing our requirements (for kind()), and not being backwards
compatible. But we should be explicit about when we are trying to do what.

My basic feeling is that 'kind()' has always been a mostly internal function,
so it is okay to break it a bit. But "id2path()" is used a lot more, so it
should be deprecated properly.

Also, if we really want a double-compatible api, then all functions should be
abstracted into this 2-layer (kind() just calls _kind()). So that people
*implementing* the api are writing a different function than people *calling*
the api.

I don't know that we really want that level of compatibility (especially when
you consider the overhead for performance). But I did want to mention it.

> 
> Ian C.
> 
> PS: The performance improvements are still pretty good so I don't want
> to hold back large gains for issues we can debate later.
> 

I would be very much in favor of a "id2path_nolock()" or some variant thereof.

I'll try to review your patch directly so that it can be merged. Unfortunately
Thunderbird 2.0 decided to fix lots of things, but also decided that
display-inline patches would not be included in replies, thereby breaking my #1
review mechanism. (They also decided that unchecking the "use-gpg-agent" box
would still supply the --use-gpg-agent flag)

I guess I just need to bug Aaron now to get BB's review box to be much bigger.
(My email editor is about 80x50-80 not 10)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGZFm3JdeBCYSNAAMRAnUiAJ9Nv7Aoi54Ut8AOtjZDLLv1174q/ACeI+0m
PryUnNwc99CnAJQTXDg0dWQ=
=4erA
-----END PGP SIGNATURE-----