From: Johannes Sixt <j6t@kdbg.org>
Date: Thu, 09 Apr 2009 20:34:38 GMT
Subject: Re: [msysGit] [PATCH] MinGW readdir reimplementation to support d_type
Message-ID: <49DE5BDE.9050709@kdbg.org>
In-Reply-To: <1239224507-5372-1-git-send-email-marius@trolltech.com>

Marius Storm-Olsen schrieb:
> The original readdir implementation was fast, but didn't
> support the d_type. This means that git would do additional
> lstats for each entry, to figure out if the entry was a
> directory or not. This unneedingly slowed down many
> operations, since Windows API provides this information
> directly when walking the directories.
> 
> By running this implementation on Moe's repo structure:
>   mkdir bummer && cd bummer; for ((i=0;i<100;i++)); do
>     mkdir $i && pushd $i;
>       for ((j=0;j<1000;j++)); do echo "$j" >$j; done;
>     popd;
>   done
> 
> We see the following speedups:
>   git add .
>   -------------------
>   old: 00:00:23(.087)
>   new: 00:00:21(.512) 1.07x
> 
>   git status
>   -------------------
>   old: 00:00:03(.306)
>   new: 00:00:01(.684) 1.96x
> 
>   git clean -dxf
>   -------------------
>   old: 00:00:01(.918)
>   new: 00:00:00(.295) 6.50x

Well done!

> +struct mingw_dirent
> +{
> +	long		d_ino;			/* Always zero. */
> +	union {
> +		unsigned short	d_reclen;	/* Always zero. */
> +		unsigned char   d_type;		/* Reimplementation adds this */
> +	};

VERY sneaky! I was wondering why you could get away without replacing 
opendir and closedir, and why you still defined a replacement mingw_DIR 
that contains the replacement mingw_dirent, until I noticed this unnamed 
union.

Since we don't use d_reclen anywhere in the code, wouldn't you get away with

#define d_type d_reclen

unless the type (short vs. char) makes a difference. Or would you say that 
doing that would be even more sneaky?

> +	unsigned short	d_namlen;		/* Length of name in d_name. */
> +	char		d_name[FILENAME_MAX];	/* File name. */
> +};
> +#define dirent mingw_dirent
> +#define readdir(x) mingw_readdir(x)
> +#endif // !NO_MINGW_REPLACE_READDIR

-- Hannes