Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit

From: Duy Nguyen <pclouds@gmail.com>
Date: Jul 8, 2014, 10:32 UTC
Message-ID: <CACsJy8C20oFdATHKTLK=9U3_kHu1QsuS4i74RPgQn0aTwVCC8w@mail.gmail.com>
In-Reply-To: <xmqq7g3obsqm.fsf@gitster.dls.corp.google.com>

On Tue, Jul 8, 2014 at 7:26 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> diff --git a/builtin/commit.c b/builtin/commit.c
>>> index 9cfef6c..5981755 100644
>>> --- a/builtin/commit.c
>>> +++ b/builtin/commit.c
>>> @@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
>>>
>>>              discard_cache();
>>>              read_cache_from(index_lock.filename);
>>> +            if (update_main_cache_tree(WRITE_TREE_SILENT) >= 0)
>>> +                    write_cache(fd, active_cache, active_nr);
>>>
>>>              commit_style = COMMIT_NORMAL;
>>>              return index_lock.filename;

I wonder if we need to update_main_cache_tree() so many times in this function. If I read the code correctly, all roads must lead to update_main_cache_tree(0) in prepare_to_commit(). If we find out that we have incomplete cache-tree before that call, we could write the index one more time after that call, instead of spreading update_main_cache_tree() all over prepare_index(). I know the "index_file" in prepare_to_commit() is probably "index.lock" or something, but that does not stop us from locking again ("index.lock.lock") if we want to update it.

Writing cache tree early in prepare_index() does help hooks, but I would say hooks are uncommon case and we could add an option to update-index to explicitly rebuild cache-tree, then hooks that do diff a lot (or other operations that use cache-tree) could rebuild cache-tree by themselves. When the index file is large, every write pays high penalty so I think trying to write less often is good.

-- 
Duy