[PATCH 0 of 3 V2] posixfile for Windows in pure Python

[Matt Mackall]

On Tue, 2011-05-17 at 14:47 +0200, Adrian Buehlmann wrote:
> On 2011-05-17 10:55, Sune Foldager wrote:
> > On 2011-05-17 00:00, Adrian Buehlmann wrote:
> >> On 2011-05-16 23:44, Matt Mackall wrote:
> >>> On Mon, 2011-05-16 at 14:23 +0200, Adrian Buehlmann wrote:
> >>>> Testing done. I've built TortoiseHg x64 msi installers with these patches applied, using
> >>>> them here myself on Windows 7 x64 (I'm sending these patches with that).
> >>>>
> >>>> Status: Cooking done. Ready for review /inclusion.
> >>>>
> >>>> Changes compared to V1:
> >>>> - minor changes in comments and change messages
> >>>> - added support for read-only attributes on class posixfile
> >>>> _______________________________________________
> >>>
> >>> I'd like to see a couple Windows/Tortoise/Pypy folks chime in on your
> >>> last patch, but I'll go ahead and queue the first two.
> >>>
> >>
> >> Yes. Definitely. I can also continue to use the third patch here to see
> >> if it does anything bad in daily use. And it's probably a good idea to
> >> give it more time. There is no need to hurry. Let's cool it down.
> > 
> > How's the performance profile for this version vs. the C version?
> 
> I do see a consistent small but measurable speed penalty with the patch
> applied to bf85c2639700 when running 'hg verify'.
> 
> I tested running from python source (of course with compiled C
> extensions, but not using a canned hg.exe) on Windows 7 x64 with x64
> Python 2.6.6.
> 
> Running time in seconds (best of ~3..5 runs, i.e. warm caches):
> 
>                                 (A)    (B)     delta
> hg verify of mercurial repo      8.34   8.47   +1.6%
> hg verify of cpython repo [1]   53.1   54.0    +1.7%

This is enough to make me wince a bit.

> (A) plain bf85c2639700
> (B) patch 3 applied to bf85c2639700
> 
> [1] http://hg.python.org/cpython/
>     9101 files, 70171 changesets, 158163 total revisions
> 
> Memory consumption seems unaffected (consistently peaks at ~118 MB when
> verifying the cpython repo).
> 
> An 'hg update' from null to tip of the cpython repo is ~21 seconds for
> both (A) and (B). I can't find a consistent speed difference for that
> use case. The variation of the running time samples seems to be larger
> than the potential difference between (A) and (B).
> 
> Looking at the big picture it is probably difficult to say if this third
> patch is really worth the risk.
> 
> Having a correct implementation of posixfile for pure is nice, but the
> question remains how relevant that is. However, posixfile = open in
> pure/osutil.py is definitely suboptimal on Windows (to say the least).

Agreed, I have no qualms about taking this into pure.

> On the other hand, we already heavily depend on things like nlinks()
> from win32.py, which already depends on ctypes now.
> 
> As a rather unrelated side note:
> 
> I measured 'hg verify' for the cpython repo using pypy.exe ([PyPy
> 1.5.0-alpha0 with MSC v.1600 32 bit] on win32) with mercurial/pure: it
> took ~170 seconds (was 54 sec when using CPython with mercurial's C
> extensions -- as noted above).

It's going to take some tuning of the pure/ code to make it competitive
with the raw C mpatch code. It's really doubtful that PyPy will ever
catch up, since the bulk of the work is here.

-- 
Mathematics is the supreme nostalgia of our time.