Message-ID: <4B2FEAA5.9020500@dbservice.com>
Date: Mon, 21 Dec 2009 22:37:41 +0100
From: Tomas Carnecky <tom@dbservice.com>
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US;
	rv:1.9.3a1pre) Gecko/20091216 Lightning/1.1a1pre Shredder/3.1a1pre
MIME-Version: 1.0
To: James Westby <jw+debian@jameswestby.net>
References: <1261315232-21494-1-git-send-email-tom@dbservice.com>
	<1261332167-17994-1-git-send-email-jw+debian@jameswestby.net>
In-Reply-To: <1261332167-17994-1-git-send-email-jw+debian@jameswestby.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: notmuch@notmuchmail.org
Subject: Re: [notmuch] [PATCH] Solaris doesn't have 'struct dirent::d_type'
Precedence: list

On 12/20/09 7:02 PM, James Westby wrote:
> From: Tomas Carnecky<tom@dbservice.com>
>
> Use stat(2) instead.
>
> Signed-off-by: Tomas Carnecky<tom@dbservice.com>
> Signed-off-by: James Westby<jw+debian@jameswestby.net>
> ---
>
>    The original patch duplicated asprintf and stat calls, rearraging
>    the code means we don't need to.
>
>    I have a concern about the duplicated stats in is_maildir, but they
>    are not so easy to save. I ran a quick timing test (3931 files), dropping
>    caches before each set:
>
>      master:
>        real  2m3.545s
>        real  1m34.571s
>        real  1m36.005s
>
>      original patch:
>        real  2m18.114s
>        real  1m34.843s
>        real  1m36.317s
>
>      revised patch:
>        real  2m5.890s
>        real  1m36.387s
>        real  1m36.453s
>
>    This shoes there is little impact of the code, but given that it is
>    around one percent we may want to make it conditional on platform
>    and save the extra stat calls.

If performance regression is an issue, something like this could be used 
to keep the current code paths in linux and stat() on other platforms:

static bool
is_dir(const char *path, struct dirent *dirent)
{
#if defined(__sun__)
	... sprintf, stat etc
#else
	(void) path;
	return dirent->d_type == DT_DIR;
#endif
}

tom