On Mon, Aug 6, 2018 at 5:53 PM, Duy Nguyen <pclo...@gmail.com> wrote: > On Sun, Aug 5, 2018 at 8:53 PM Christian Couder > <christian.cou...@gmail.com> wrote: >> >> As you can see the patch 6/6 (in the v2 of this patch series that I >> just sent) moves `unsigned int tree_depth` from 'struct object_entry' >> to 'struct packing_data'. I am not sure that I did it right and that >> it is worth it though as it is a bit more complex. > > It is a bit more complex than I expected. But I think if you go with > Jeff's suggestion (i.e. think of the new tree_depth array an extension > of objects array) it's a bit simpler: you access both arrays using the > same index, both arrays should have the same size and be realloc'd at > the same time in packlist_alloc().
Ok, I will take a look at doing that to simplify things. Thanks to Peff and you for that suggestion! > Is it worth it? The "pahole" comment in this file is up to date. We > use 80 bytes per object. This series makes the struct 88 bytes (I've > just rerun pahole). Did you run it on V1 or on V2? I guess on V2, but then what do you think about converting the 'layer' field into a bit field, which might be simpler and save space? > On linux repo with 12M objects, "git pack-objects > --all" needs extra 96MB memory even if this feature is not used. So > yes I still think it's worth moving these fields out of struct > object_entry. And what about the fields already in struct object_entry? While I am at it, I think I could move some of them too if it is really so worth it.
