Re: [PATCH v3 17/24] read-cache: read cache-tree in index-v5

From: Duy Nguyen <pclouds@gmail.com>
Date: Aug 24, 2013, 00:09 UTC
Message-ID: <CACsJy8Br8EaEtoZeaz2eLKvxDmPJqUc02bYsL_OSLJvJ2dBU-g@mail.gmail.com>
In-Reply-To: <1376854933-31241-18-git-send-email-t.gummerer@gmail.com>

On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:

> +/*
> + * This function modifies the directory argument that is given to it.
> + * Don't use it if the directory entries are still needed after.
> + */

There goes my hope of keeping directory_entry* in core so that at write-time, after validation, we only need to recreate some trees instead of all of them..

Or we could make cache-tree keep references to directory_entry. If a cache-tree is not invalidated, then the attached directory_tree should be reused..

Show 19 quoted lines

> +static struct cache_tree *cache_tree_convert_v5(struct directory_entry *de)
> +{
> +       if (!de->de_nentries)
> +               return NULL;
> +       sort_directories(de);
> +       return convert_one(de);
> +}
> +
>  static int read_entries(struct index_state *istate, struct directory_entry *de,
>                         unsigned int first_entry_offset, void *mmap,
>                         unsigned long mmap_size, unsigned int *nr,
> @@ -591,6 +668,7 @@ static int read_index_v5(struct index_state *istate, void *mmap,
>                 }
>                 de = de->next;
>         }
> +       istate->cache_tree = cache_tree_convert_v5(root_directory);
>         istate->cache_nr = nr;
>         return 0;
>  }

Otherwise we do need to free root_directory down to the deepest subtrees, I think. People have been complaining about read-cache leaking memory like mad, so this is a real issue. Even if you keep references in cache-tree, you still need to free it cache_tree_invalidate_path() to avoid leaking

-- 
Duy