[MERGE] inode sorted stats in status

[Robert Collins]

On Wed, 2008-08-20 at 08:12 -0500, John Arbash Meinel wrote:


> +    ctypedef struct dirent:
> +        char d_name[256]
> 
> ^- you declare 'dirent' as a typedef here, and then have to force one later.
> Why not just do:
> 
> struct dirent:
> ?

It fails? At least back with the pyrex I wrote this on - this was the
first pyrex branch for bzr :).

> Alternatively, you could do:
> 
> struct _dirent:
> ...
> 
> ctypedef _dirent dirent
> 
> I get the feeling you are running into typedef problems because you explicitly
> declared it as a typedef when it is just a plain struct object.
> 
> +    ctypedef struct DIR
> +    # should be DIR *, pyrex barfs.
> +    DIR * opendir(char * name)
> 
> ^- I don't understand the comment here.

Hmm, that is strange; I'll poke at it some.

> 
> +dot = ord('.')
> +
> 
> ^- I think what you are looking for is c'.' which is the pyrex way of saying
> you are talking about a single character.

thanks.

> I would certainly recommend taking over the *whole* walkdir stack, like I did
> with the _walkdirs_win32 extension. To do so, you have to create an iterator
> class (that defines __next__), because pyrex doesn't support generators.
> 
> However, it ends up being very nice because you can have context and
> functions. We should really consider changing the _walkdirs_utf8 api to be an
> iterator class which allows you to call "skip(directory)" rather than the
> current api. IIRC, you wanted this for handling the mixing of WT.walkdirs()
> which had 2 iterators it needed to handle properly.

Yes. I'm actually thinking of different ways to achieve that. Right now,
its about 0.1 seconds or 9% of iteration spent handling the
skip/not-skip logic. I'm considering just exposing the additional queue
for the next directory as a list. Then the client can alter it as
needed. As far as stat performance goes, I'm currently updating my
path_info branch, and we'll see how that goes.

> So in the end, I'd like to see a bigger improvement before we bring this in. I
> think you are probably on the right track, but 2s out of 35s isn't really
> worth it yet.

If the code is solid, this is something I feel safe bringing in in small
steps; I'd rather not save up a 'big bang' patch unless there is doubt
that the end result would be merged, or that a particular step is in a
good direction. Your review didn't seem to indicate either thing.

-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/20080821/43917b46/attachment.pgp