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
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