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

2013-11-25 Thread Thomas Gummerer
Duy Nguyen pclo...@gmail.com writes:

 On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer t.gumme...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-23 Thread Duy Nguyen
On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer t.gumme...@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..

 +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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html