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

[Thomas Gummerer <t.gummerer@gmail.com>]

Duy Nguyen <pclouds@gmail.com> writes:

> 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..

I've now re-written the algorithm that converts the directory entries to
cache entries, and it's now no longer destructive.  For now the
directory entries are not needed in core, so I'll free them when done
with reading the index, but it's possible to keep it.

>> +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

I'm freeing them for now, as they are not used anywhere, but in the
future we might want to keep them for some optimizations.