DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma;
	h=mime-version:sender:in-reply-to:references:date
	:x-google-sender-auth:message-id:subject:from:to:cc:content-type
	:content-transfer-encoding;
	b=RppbrDi3j7v92MXvUtv4w99I6unzHUgVj0h0aKEkuVVmJdO86p6lfUg8btXjJyJrFo
	DTMbEq5e53vWTvVmMobiJG5bpLMXUl2y2QQvGaH7UL52k4akgzwXITOq/Iq3F9TK38/3
	ph7pJh1x8vG6oGs0eWwhIvBwZmZOoedeVbgxA=
MIME-Version: 1.0
Sender: amdragon@gmail.com
In-Reply-To: <1296855871-15702-1-git-send-email-kzak@redhat.com>
References: <1296855871-15702-1-git-send-email-kzak@redhat.com>
Date: Sun, 27 Feb 2011 03:45:05 -0500
Message-ID: <AANLkTi=PfbB9Fv6euxuzsjOkSsj-EqQxTFuz9ucfzJHi@mail.gmail.com>
Subject: Re: [PATCH] new: read db_files and db_subdirs if mtime changed
From: Austin Clements <amdragon@mit.edu>
To: Karel Zak <kzak@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: notmuch@notmuchmail.org
Precedence: list

Looks good (faster than, but provably equivalent to the original code!
 notmuch_directory_get_child_* are side-effect free,
db_files/db_subdirs aren't used between where they were set in the old
code and where they are set in the new code, and db_files/db_subdirs
are initialized to NULL when declared).

Another timing data point:
Old code: ./notmuch new  0.77s user 0.28s system 99% cpu 1.051 total
New code: ./notmuch new  0.09s user 0.27s system 98% cpu 0.368 total

I wonder if an even faster approach than the current recursive walk
would be to get *all* of the directory names and mtimes out of Xapian
in one pass and stat them all.  If the mtime didn't change, then
there's no need to scandir that directory at all.  This could even
beat the "time find >/dev/null" bound, but the gains may be too
marginal to make it worthwhile.

On Fri, Feb 4, 2011 at 4:44 PM, Karel Zak <kzak@redhat.com> wrote:
> The db_files and db_subdirs are unnecessary for unchanged directories.
>
> maildir with 10000 e-mails:
>
> old version:
> =A0 =A0 =A0 =A0$ time ./notmuch new
> =A0 =A0 =A0 =A0No new mail.
>
> =A0 =A0 =A0 =A0real =A0 =A00m0.053s
> =A0 =A0 =A0 =A0user =A0 =A00m0.028s
> =A0 =A0 =A0 =A0sys =A0 =A0 0m0.026s
>
> new version:
> =A0 =A0 =A0 =A0$ time ./notmuch new
> =A0 =A0 =A0 =A0No new mail.
>
> =A0 =A0 =A0 =A0real =A0 =A00m0.032s
> =A0 =A0 =A0 =A0user =A0 =A00m0.009s
> =A0 =A0 =A0 =A0sys =A0 =A0 0m0.023s
>
> Signed-off-by: Karel Zak <kzak@redhat.com>
> ---
> =A0notmuch-new.c | =A0 15 ++++++---------
> =A01 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 941f9d6..31d4553 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -247,15 +247,7 @@ add_files_recursive (notmuch_database_t *notmuch,
> =A0 =A0 directory =3D notmuch_database_get_directory (notmuch, path);
> =A0 =A0 db_mtime =3D notmuch_directory_get_mtime (directory);
>
> - =A0 =A0if (db_mtime =3D=3D 0) {
> - =A0 =A0 =A0 new_directory =3D TRUE;
> - =A0 =A0 =A0 db_files =3D NULL;
> - =A0 =A0 =A0 db_subdirs =3D NULL;
> - =A0 =A0} else {
> - =A0 =A0 =A0 new_directory =3D FALSE;
> - =A0 =A0 =A0 db_files =3D notmuch_directory_get_child_files (directory);
> - =A0 =A0 =A0 db_subdirs =3D notmuch_directory_get_child_directories (dir=
ectory);
> - =A0 =A0}
> + =A0 =A0new_directory =3D db_mtime ? FALSE : TRUE;
>
> =A0 =A0 /* If the database knows about this directory, then we sort based
> =A0 =A0 =A0* on strcmp to match the database sorting. Otherwise, we can d=
o
> @@ -328,6 +320,11 @@ add_files_recursive (notmuch_database_t *notmuch,
> =A0 =A0 if (fs_mtime =3D=3D db_mtime)
> =A0 =A0 =A0 =A0goto DONE;
>
> + =A0 =A0if (!new_directory) {
> + =A0 =A0 =A0 db_files =3D notmuch_directory_get_child_files (directory);
> + =A0 =A0 =A0 db_subdirs =3D notmuch_directory_get_child_directories (dir=
ectory);
> + =A0 =A0}
> +
> =A0 =A0 /* Pass 2: Scan for new files, removed files, and removed directo=
ries. */
> =A0 =A0 for (i =3D 0; i < num_fs_entries; i++)
> =A0 =A0 {
> --
> 1.7.3.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>