Re: [PATCH v4 08/16] Add watchman support to reduce index refresh cost

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 17, 2016, 05:36 UTC
Message-ID: <CACsJy8CbQfFdv8+v769zxzySTfkUcneakMLzzxNWFpKS-nF4gA@mail.gmail.com>
In-Reply-To: <1460507589-25525-9-git-send-email-dturner@twopensource.com>

On Wed, Apr 13, 2016 at 7:33 AM, David Turner <dturner@twopensource.com> wrote:

Show 48 quoted lines

> +static void update_index(struct index_state *istate,
> +                        struct watchman_query_result *result)
> +{
> +       int i;
> +
> +       if (result->is_fresh_instance) {
> +               /* let refresh clear them later */
> +               for (i = 0; i < istate->cache_nr; i++)
> +                       istate->cache[i]->ce_flags |= CE_WATCHMAN_DIRTY;
> +               goto done;
> +       }
> +
> +       for (i = 0; i < result->nr; i++) {
> +               struct watchman_stat *wm = result->stats + i;
> +               int pos;
> +
> +               if (S_ISDIR(wm->mode) ||
> +                   !strncmp(wm->name, ".git/", 5) ||
> +                   strstr(wm->name, "/.git/"))
> +                       continue;
> +
> +               pos = index_name_pos(istate, wm->name, strlen(wm->name));
> +               if (pos < 0) {
> +                       if (istate->untracked) {
> +                               char *name = xstrdup(wm->name);
> +                               char *dname = dirname(name);
> +
> +                               /*
> +                                * dirname() returns '.' for the root,
> +                                * but we call it ''.
> +                                */
> +                               if (dname[0] == '.' && dname[1] == 0)
> +                                       string_list_append(&istate->untracked->invalid_untracked, "");
> +                               else
> +                                       string_list_append(&istate->untracked->invalid_untracked,
> +                                                          dname);
> +                               free(name);
> +                       }
> +                       continue;
> +               }
> +               /* FIXME: ignore staged entries and gitlinks too? */
> +
> +               istate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;
> +       }
> +
> +done:
> +       free(istate->last_update);
> +       istate->last_update    = xstrdup(result->clock);

I made a note in private that, because we update this field every time, the index must be rewritten every time with new WAMA extension, which results in worse performance when the index file is big and update cost is really high (SHA-1 hashing again).

What I did not notice was, if we detect no changes since the last clock here (result->nr == 0 _or_ no new entries marked DIRTY), we could skip this clock update here. That results in no changes in index (on read-cache side), so no index update, no hashing cost.

-- 
Duy