[PATCH] context: don't hex encode all unknown 20 char revision specs (issue4890)

[Mads Kiilerich]

On 10/11/2015 05:57 PM, Matt Mackall wrote:
> On Fri, 2015-10-09 at 12:57 +0200, Mads Kiilerich wrote:
>>>> -                if len(changeid) == 20:
>>>> +                if len(changeid) == 20 and nonascii(changeid):
>>>>                        changeid = hex(changeid)
>>> You cannot put a regex check in the context creation fast path. That
>>> code desperately needs to be faster, not slower.
>>>
>>> You are, however, welcome to make the error handler as slow as you want.
>> This _is_ in the error handler.
> Right, now it all makes sense. I thought this was the earlier "== 20"
> block. Queued for default, thanks.

I looked more at it and also agree that the end of the __init__ not 
_just_ is an _error_ handler. It is also reached from localrepo 
__contains__ and lookup where RepoLookupError is caught and the nice 
localized message is discarded.

Some anecdotal evidence from the test suite: it only ends up in the case 
with missing 20 char changeids 50 times. 37 of them are 
'_rebasedefaultdest()'. Only 7 of them are binary. The code did thus 
already have room for improvement and the extra regexp check is not in a 
fast fast path and will not slow anything down in a noticeable way.

Also agreed, as a part of the mentioned desperately needed improvement, 
these nice and often irrelevant messages could be moved to the "real" 
error handler in dispatch.

/Mads
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151012/59886c92/attachment.html>