Web lists-archives.com

Re: [PATCH 00/12] Die commit->util, die!




On Sat, May 12, 2018 at 10:00:16AM +0200, Nguyễn Thái Ngọc Duy wrote:

> There's not much to write here. It's basically a copy from 12/12:
> 
> This 'util' pointer can be used for many different purposes,
> controlled in different ways. Some are not even contained in a command
> code, but buried deep in common code with no clue who will use it and
> how. For example, if revs.show_source is set, then it's used for
> storing path name, but if you happen to call get_merge_parent() then
> some 'util' may end up storing another thing.
> 
> The move to using commit-slab gives us a much better picture of how
> some piece of data is associated with a commit and what for. Since
> nobody uses 'util' pointer anymore, we can retire it so that nobody will
> abuse it again. commit-slab will be the way forward for associating
> data to a commit.
> 
> As a side benefit, this shrinks struct commit by 8 bytes (on 64-bit
> architecture) which should help reduce memory usage for reachability
> test a bit. This is also what commit-slab is invented for [1].

I left a few comments, but overall this looks pretty good. A few of the
conversions get tricky with the number of pointer dereferences, but most
of those were pretty tricky to begin with (that weight stuff in
bisect.c...yikes!).

I love the result. More maintainable code, less possibility of conflicts
in the util field, and a memory savings to boot.

-Peff