Web lists-archives.com

Re: [PATCH v2 14/14] commit.h: delete 'util' field in struct commit




On 5/14/2018 1:30 PM, Duy Nguyen wrote:
On Mon, May 14, 2018 at 6:07 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
On Mon, May 14, 2018 at 04:52:29PM +0900, Junio C Hamano wrote:
Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

diff --git a/commit.h b/commit.h
index 838f6a6b26..70371e111e 100644
--- a/commit.h
+++ b/commit.h
@@ -18,12 +18,16 @@ struct commit_list {

  struct commit {
     struct object object;
-   void *util;
     unsigned int index;
     timestamp_t date;
     struct commit_list *parents;
     struct tree *tree;
     uint32_t graph_pos;
+   /*
+    * Do not add more fields here unless it's _very_ often
+    * used. Use commit-slab to associate more data with a commit
+    * instead.
+    */
  };
That's a logical consequence and a natural endgame of this
pleasent-to-read series.  THanks.

Unfortunately we are gaining more and more stuff in "struct commit"
with recent topics, and we may want to see if we can evict some of
them out to shrink it again.
Sigh.. ds/lazy-load-trees already enters 'next' so a fixup patch is
something like this.
Sorry I take this patch back. I didn't realize it was a rename and the
old field named 'tree' was already there (I vaguely recalled some
"tree" in this struct but didn't stop to think about it). Moving
graph_pos out is an option, but only 32-bit arch gains from it (64-bit
arch will have 4 bytes padding anyway) so probably not worth the
effort. "generation" field should probably be moved out though.

I'm happy to take a look at this patch and figure out the best way to integrate it with the changes I've been doing.

I disagree with the removal of "generation". My intention is to make the commit-graph feature a standard feature that most repos (of reasonable size) want to have. In that case, 'graph_pos' and 'generation' will be set during every parse and used by every commit walk. This is just my gut reaction.

As I investigate these changes, I'll try to see what performance hit is caused by converting the graph_pos and/or generation to commit slabs. (Again, I'm assuming the slabs will make this slower. I may be wrong here.)

Thanks,
-Stolee