[PATCH 07 of 10] util: implement a faster os.path.split for posix systems

Adrian Buehlmann adrian at cadifra.com
Fri Sep 14 01:56:06 CDT 2012


On 2012-09-14 03:25, Greg Ward wrote:
> On 13 September 2012, Bryan O'Sullivan said:
>> # HG changeset patch
>> # User Bryan O'Sullivan <bryano at fb.com>
>> # Date 1347562630 25200
>> # Node ID 9d1ceb24dc23a965d6df2b14ebca30da59139710
>> # Parent  38159ce2f0115498a0c43b4c84a3f9ba87d08561
>> util: implement a faster os.path.split for posix systems
> 
> Measurable speedup?
> 
>> diff --git a/mercurial/posix.py b/mercurial/posix.py
>> --- a/mercurial/posix.py
>> +++ b/mercurial/posix.py
>> @@ -20,6 +20,16 @@
>>  umask = os.umask(0)
>>  os.umask(umask)
>>  
>> +def split(p):
>> +    '''Same as os.path.split, but faster'''
>> +    ht = p.rsplit('/', 1)
>> +    if len(ht) == 1:
>> +        return '', p
>> +    nh = ht[0].rstrip('/')
>> +    if nh:
>> +        return nh, ht[1]
>> +    return ht
>> +
> 
> -1 on the cryptic variable names. If "ht" means "headtail", why not
> say "headtail"? It really does make things easier to read.

Like it or not, but I think these short local variable names are fine
and are in the spirit of Mercurial's coding style, which I think we
shouldn't start questioning each time such a thing pops up. It's not
healthy.

FWIW, I think it's actually less risk for typos (especially in an
interpreted language), and those locals are referenced frequently enough.

> And this kind of thing is The Ultimate Poster Child for unit testing.
> Yeah, yeah, I know, Mercurial doesn't do unit testing. But some of
> those test-*.py scripts are an awful lot like unit testing, so
> apparently it's not completely verboten. Time for test-posix.py?

Perhaps a doctest might do it.



More information about the Mercurial-devel mailing list