[PATCH 07 of 10] util: implement a faster os.path.split for posix systems
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)
>> +def split(p):
>> + '''Same as os.path.split, but faster'''
>> + ht = p.rsplit('/', 1)
>> + if len(ht) == 1:
>> + return '', p
>> + nh = ht.rstrip('/')
>> + if nh:
>> + return nh, ht
>> + 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
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