[RFC/MERGE] Implement DirState parsing in Pyrex

[Robert Collins]

On Mon, 2007-05-07 at 18:37 -0500, John Arbash Meinel wrote:
> 
> The main reason this is RFC is because it is introducing a (potential)
> dependency on compiled code. I tried to make sure it was genuinely
> worth
> it before I advocated it. But I think our disk format is fairly stable
> (and the parser is actually easier to modify in pyrex because of the
> lack of hacks).

I agree.

> Originally, I know Robert was thinking to version the .c file, along
> with the .pyx files. However, this will cause a lot of commit churn,
> because when I build them it includes my system paths
> (/home/jameinel/...foo/bar), and when the PQM builds them, it will
> create different paths there.
> 
> What I would rather do, is change our *release* process so that we
> include them in the final tar file.

I dont like this, it means that tarballs will be easier to install from
than our source tree. I'd rather fix the .c source - e.g. post process
it to have relative paths. (Absolute paths are not needed AFAIK).

> That would mean that people who are using bzr.dev directly would
> either:
> 
> 1) Use the python implementations
> 2) Need to have pyrex installed (and gcc, etc)
> 
> People who have release versions would
> 
> 1) Use the python implementations
> 2) Need to have python-dev and gcc (etc) installed.
> 
> Since a lot of people use "apt-get install bzr" having a compile step
> isn't as terrible as it used to be. And as long as we follow the "it
> will always work on plain python, just be faster if you have a
> compiler", I think I'm happy.
> 
> 
> Ultimately, I think we want to move pyrex up the stack into
> _iter_changes. Some care will be necessary, because pyrex doesn't
> support generators. But I think we can write some of the
> _process_entry
> code in pyrex, which should help to speed up 'bzr status', etc.

Sure.

> One other possibility is to have a 'bzr.dev-pyrex' branch. Which would
> let us add things like this on the side, and only merge them once we
> have decided it is worthwhile.

I dont think having a separate 'official' branch makes sense: Either its
good enough or its not :).

> Also, for now I've gone with writing functions such that both the
> python
> code and the pyrex code are tested by ./bzr selftest. Rather than
> having
> 'make check' run one time without them, and one time with them.

I like this.

> Oh, a few other things...
> 
> I was thinking to put all of the extension code in one place (I picked
> bzrlib/compiled/*). Part of that is we could theoretically have a "bzr
> --no-compiled" flag. It might also make it easier to have
> CompiledFeature in bzrlib/compiled/__init__.py.
> 
> I also made the matching tests bzrlib/tests/compiled/*
> 
> This means that I use local variables to select the implementation
> (bzrlib/dirstate.py has a '_read_dirblocks' function which points to
> either _read_dirblocks_py or _read_dirblocks_c).
> 
> I know Robert mentioned having the .pyx right next to the .py, so that
> the compiled .so would have the same name. And then 'import
> bzrlib.foo'
> would pick between foo.so or foo.py depending on if it had been
> compiled.

Personally I'd prefer to see the .pyx and .py in the same place - e.g.
dirstate_core.{c,py,pyx} and in dirstate you do from dirstate_core
import *. Or you can look at my readdir pyrex branch to see how I've
done it there.

> This also gets my feet a bit wet on writing (fast) pyrex code. One
> interesting part is that the intermediate C code allows you to see how
> your code is being compiled. Sort of like reading the assembly output
> for C code, only I don't have to read assembly :).
> It has helped in a few cases where I didn't realize the overhead of
> one type of function. 

Generally I'm +1 on the concept, though I haven't read the detail of the
patch yet, and I'd like use to reach agreement on the layout stuff
before its merged.

Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070508/28761e5c/attachment.pgp