Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions

[Stefan Beller <sbeller@google.com>]

On Wed, May 2, 2018 at 10:44 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, May 1, 2018 at 11:34 PM, Stefan Beller <sbeller@google.com> wrote:
>>  #include "cache.h"
>>  #include "object.h"
>> @@ -30,8 +31,25 @@ struct alloc_state {
>>         int count; /* total number of nodes allocated */
>>         int nr;    /* number of nodes left in current allocation */
>>         void *p;   /* first free node in current allocation */
>> +
>> +       /* bookkeeping of allocations */
>> +       void **slabs;
>
> Another way to manage this is linked list: you could reserve one
> "object" in each slab to store the "next" (or "prev") pointer to
> another slab, then you can just walk through all slabs and free. It's
> a bit cheaper than reallocating slabs[], but I guess we reallocate so
> few times that readability matters more (whichever way is chosen).

This is a good idea. I'll do so in a resend.

>> +void clear_alloc_state(struct alloc_state *s)
>> +{
>> +       while (s->slab_nr > 0) {
>> +               s->slab_nr--;
>> +               free(s->slabs[s->slab_nr]);
>
> I think you're leaking memory here. Commit and tree objects may have
> more allocations in them (especially trees, but I think we have
> commit_list in struct commit too). Those need to be freed as well.

I would think that tree and commit memory will be free'd in the
parsed_objects release function? (TODO: I need to add it over there)

Thanks,
Stefan