Re: [PATCH] new: read db_files and db_subdirs if mtime changed

[Austin Clements <amdragon@mit.edu>]

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:
>        $ time ./notmuch new
>        No new mail.
>
>        real    0m0.053s
>        user    0m0.028s
>        sys     0m0.026s
>
> new version:
>        $ time ./notmuch new
>        No new mail.
>
>        real    0m0.032s
>        user    0m0.009s
>        sys     0m0.023s
>
> Signed-off-by: Karel Zak <kzak@redhat.com>
> ---
>  notmuch-new.c |   15 ++++++---------
>  1 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,
>     directory = notmuch_database_get_directory (notmuch, path);
>     db_mtime = notmuch_directory_get_mtime (directory);
>
> -    if (db_mtime == 0) {
> -       new_directory = TRUE;
> -       db_files = NULL;
> -       db_subdirs = NULL;
> -    } else {
> -       new_directory = FALSE;
> -       db_files = notmuch_directory_get_child_files (directory);
> -       db_subdirs = notmuch_directory_get_child_directories (directory);
> -    }
> +    new_directory = db_mtime ? FALSE : TRUE;
>
>     /* If the database knows about this directory, then we sort based
>      * on strcmp to match the database sorting. Otherwise, we can do
> @@ -328,6 +320,11 @@ add_files_recursive (notmuch_database_t *notmuch,
>     if (fs_mtime == db_mtime)
>        goto DONE;
>
> +    if (!new_directory) {
> +       db_files = notmuch_directory_get_child_files (directory);
> +       db_subdirs = notmuch_directory_get_child_directories (directory);
> +    }
> +
>     /* Pass 2: Scan for new files, removed files, and removed directories. */
>     for (i = 0; i < num_fs_entries; i++)
>     {
> --
> 1.7.3.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>