On 11/07/14 09:32, Jeff King wrote: > On Fri, Jul 11, 2014 at 01:59:53AM +0100, Ramsay Jones wrote: > >>> The code you're touching here was trying to make sure that each commit >>> gets a unique index, under the assumption that commits only get >>> allocated via alloc_commit_node. But I think that assumption is wrong. >>> We can also get commit objects by allocating an OBJ_NONE (e.g., via >>> lookup_unknown_object) and then converting it into an OBJ_COMMIT when we >>> find out what it is. >> >> Hmm, I don't know how the object is converted, but the object allocator >> is actually allocating an 'union any_object', so it's allocating more >> space than for a struct object anyway. > > Right, we would generally want to avoid lookup_unknown_object where we > can for that reason. > >> If you add an 'index' field to struct object, (and remove it from >> struct commit) it could be set in alloc_object_node(). ie _all_ node >> types get an index field. > > That was something I considered when we did the original commit-slab > work, as it would let you do similar tricks for any set of objects, not > just commits. The reasons against it are: > > 1. It would bloat the size of blob and tree structs by at least 4 > bytes (probably 8 for alignment). In most repos, commits make up > only 10-20% of the total objects (so for linux.git, we're talking > about 25MB extra in the working set). > > 2. It makes single types sparse in the index space. In cases where you > do just want to keep data on commits (and that is the main use), > you end up allocating a slab entry per object, rather than per > commit. That wastes memory (much worse than 25MB if your slab items > are large), and reduces cache locality. >
Ahem, yeah I keep telling myself not to post at 2am ... ;-) Although I haven't given this too much extra thought, I'm beginning to think that your best course would be to revert commit 969eba63 and add an convert_object_to_commit() function to commit.c which would set c->index. Then track down each place an OBJ_NONE gets converted to an OBJ_COMMIT and insert a call to convert_object_to_commit(). (which may do more than just set the index field ...) Hmm, I've just noticed a new series in my in-box. I guess I will discover what you decided to do shortly ... ;-P ATB, Ramsay Jones -- 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
