Re: rev-list/tree committer/author information.

From: Daniel Barkalow <barkalow@iabervon.org>
Date: May 16, 2005, 22:35 UTC
Message-ID: <Pine.LNX.4.21.0505161747290.30848-100000@iabervon.org>
In-Reply-To: <7vekc6onzc.fsf_-_@assigned-by-dhcp.cox.net>

On Mon, 16 May 2005, Junio C Hamano wrote:

Show 11 quoted lines

> The only reason why I did it that way was because the strategy
> taken by "struct object" derivatives seemed to pick up bare
> absolute minimum to support actual callers that have immediate
> need for information stored in structural fields, as opposed to
> designing for helping yet to be written callers by adding fields
> to hold information of "having this might also help somebody in
> the future" type.  And the author and committer names are in the
> structured fields while signed-off-by and others are not.  Also
> when author / committer name strings are intern'ed like the way
> I did, the memory consumption even for a long sequence of
> commits are kept reasonably low.  However,...

I think if we had a good reason to make author and committer structured, there must be some reason we'll want them parsed out eventually, although we may not have reached the point at which it makes sense to do it.

Show 8 quoted lines

> The above implies to keep the unpacked raw data as a whole to be
> accessible to the callers for at least commit objects and if we
> go that route I think it would make more sense to do that
> uniformly for everything (probably except for pure "blob"
> objects for size concerns but we might as well do them while we
> are at it).  On the other hand, the current lifetime rules being
> what it is, that strategy may introduce memory consumption
> problems when working on a huge project.

Perhaps have a struct object field for the unpacked data, have it filled by a unpack_object(struct object *) function, have the parse functions use it if it's filled, and have a function to free it (and NULL the field).

So, if you want the actual contents, you'd call unpack_object(&commit->obj) before parse_commit(commit), and close_object(&commit_obj) when you're done, and the parse wouldn't duplicate the work. The lifetime rules only really care that the return value from lookup_*() stays the same for the duration of the program, not that the fields in it don't get cleared (aside from the flags and the hash).

This could be useful even for blobs; I'm working on a program to do diff/merge on commits and everything under them (taking advantage of all available information if the fast path leads to a conflict); it would be nice not to have to go through the filesystem with the blob data.

	-Daniel
*This .sig left intentionally blank*