On Wed, May 9, 2018 at 7:50 PM, Stefan Beller <sbel...@google.com> wrote: >> I was trying to test the new parsed_object_pool_clear() and found this. > > So this would go with the latest sb/object-store-alloc ?
No this should be separate because sb/object-store-alloc did not even touch this code. I mistakenly thought you wrote this and sent to you. I should have checked and sent to Brandon instead. > My impression was that we never call repo_clear() on > the_repository, which would allow us to special case > the_repository further just as I did in v2 of that series[1] by > having static allocations for certain objects in case of \ > the_repository. > > [1] https://public-inbox.org/git/20180501213403.14643-14-sbel...@google.com/ > > We could just deal with all the exceptions, but that makes repo_clear > ugly IMHO. > > I would rather special case the_repository even more instead > of having it allocate all its things on the heap. (However we rather > want to profile it and argue with data....) I'm actually going the opposite direction and trying hard to make the_repository normal like everybody else :) >> repository.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/repository.c b/repository.c >> index a4848c1bd0..f44733524a 100644 >> --- a/repository.c >> +++ b/repository.c >> @@ -238,7 +238,9 @@ void repo_clear(struct repository *repo) >> >> if (repo->index) { >> discard_index(repo->index); >> - FREE_AND_NULL(repo->index); >> + if (repo->index != &the_index) >> + free(repo->index); >> + repo->index = NULL; > > So after this we have a "dangling" the_index. > It is not really dangling, but it is not part of the_repository any more > and many places still use the_index, it might make up for interesting > bugs? Hmm.. yeah, as a "clearing" operation, I probaly should not clear repo->index. This way the_repository will be back in initial state and could be reused again. Something like this perhaps? discard_index(repo->index); if (repo->index != &the_index) FREE_AND_NULL(repo->index); > What is your use case of repo_clear(the_repository)? No actual use case right now. See [1] for the code that triggered this. I do want to free some memory in pack-objects and repo_clear() _might_ be the one. I'm not sure yet. [1] https://gist.github.com/pclouds/86a2df6c28043f1b6fa3d4e72e7a1276 -- Duy
