git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
May 14, 2018, 16:07 UTC
Message-ID
<20180514160738.GA19821@duynguyen.home>
In-Reply-To
<xmqqy3gmbrnm.fsf@gitster-ct.c.googlers.com>
On Mon, May 14, 2018 at 04:52:29PM +0900, Junio C Hamano wrote:
Show 29 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> 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.

Derrick, do you want to finish up this patch? I could do that by myself, but I guess this could be good for you to get familiar with commit-slab because ds/generation-numbers on 'pu' also adds a new field in struct commit.

We should avoid adding more stuff in struct commit to reduce overhead when these fields are not used (and I'm not sure how often generation numbers are used to justify its place here). One clean way to do it is with commit-slab, as demonstrated here for 'maybe_tree' field.

Anyway the patch is like this
-- 8< --
diff --git a/commit-graph.c b/commit-graph.c
index 70fa1b25fd..9d084f6200 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -9,6 +9,7 @@
 #include "revision.h"
 #include "sha1-lookup.h"
 #include "commit-graph.h"
+#include "commit-slab.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
@@ -38,6 +39,26 @@
 #define GRAPH_MIN_SIZE (5 * GRAPH_CHUNKLOOKUP_WIDTH + GRAPH_FANOUT_SIZE + \
 			GRAPH_OID_LEN + 8)
 
+
+/*
+ * If the commit is loaded from the commit-graph file, then this
+ * member may be NULL. Only access it through get_commit_tree()
+ * or get_commit_tree_oid().
+ */
+define_commit_slab(maybe_tree, struct tree *);
+struct maybe_tree maybe_trees = COMMIT_SLAB_INIT(1, maybe_trees);
+
+struct tree *set_maybe_tree(const struct commit *commit, struct tree *tree)
+{
+	*maybe_tree_at(&maybe_trees, commit) = tree;
+	return tree;
+}
+
+struct tree *get_maybe_tree(const struct commit *commit)
+{
+	return *maybe_tree_at(&maybe_trees, commit);
+}
+
 char *get_commit_graph_filename(const char *obj_dir)
 {
 	return xstrfmt("%s/info/commit-graph", obj_dir);
@@ -256,7 +277,7 @@ static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, uin
 	item->object.parsed = 1;
 	item->graph_pos = pos;
 
-	item->maybe_tree = NULL;
+	set_maybe_tree(item, NULL);
 
 	date_high = get_be32(commit_data + g->hash_len + 8) & 0x3;
 	date_low = get_be32(commit_data + g->hash_len + 12);
@@ -322,15 +343,15 @@ static struct tree *load_tree_for_commit(struct commit_graph *g, struct commit *
 					   GRAPH_DATA_WIDTH * (c->graph_pos);
 
 	hashcpy(oid.hash, commit_data);
-	c->maybe_tree = lookup_tree(&oid);
-
-	return c->maybe_tree;
+	return set_maybe_tree(c, lookup_tree(&oid));
 }
 
 struct tree *get_commit_tree_in_graph(const struct commit *c)
 {
-	if (c->maybe_tree)
-		return c->maybe_tree;
+	struct tree *maybe_tree = get_maybe_tree(c);
+
+	if (maybe_tree)
+		return maybe_tree;
 	if (c->graph_pos == COMMIT_NOT_FROM_GRAPH)
 		BUG("get_commit_tree_in_graph called from non-commit-graph commit");
 
diff --git a/commit-graph.h b/commit-graph.h
index 260a468e73..4a02e4b05e 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -45,4 +45,7 @@ void write_commit_graph(const char *obj_dir,
 			int nr_commits,
 			int append);
 
+struct tree *set_maybe_tree(const struct commit *commit, struct tree *tree);
+struct tree *get_maybe_tree(const struct commit *commit);
+
 #endif
diff --git a/commit.c b/commit.c
index 711f674c18..45521ab2de 100644
--- a/commit.c
+++ b/commit.c
@@ -298,8 +298,10 @@ void free_commit_buffer(struct commit *commit)
 
 struct tree *get_commit_tree(const struct commit *commit)
 {
-	if (commit->maybe_tree || !commit->object.parsed)
-		return commit->maybe_tree;
+	struct tree *maybe_tree = get_maybe_tree(commit);
+
+	if (maybe_tree || !commit->object.parsed)
+		return maybe_tree;
 
 	if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
 		BUG("commit has NULL tree, but was not loaded from commit-graph");
@@ -351,7 +353,7 @@ int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long s
 	if (get_sha1_hex(bufptr + 5, parent.hash) < 0)
 		return error("bad tree pointer in commit %s",
 			     oid_to_hex(&item->object.oid));
-	item->maybe_tree = lookup_tree(&parent);
+	set_maybe_tree(item, lookup_tree(&parent));
 	bufptr += tree_entry_len + 1; /* "tree " + "hex sha1" + "\n" */
 	pptr = &item->parents;
 
diff --git a/commit.h b/commit.h
index 23a3f364ed..b7274ca365 100644
--- a/commit.h
+++ b/commit.h
@@ -22,13 +22,6 @@ struct commit {
 	unsigned int index;
 	timestamp_t date;
 	struct commit_list *parents;
-
-	/*
-	 * If the commit is loaded from the commit-graph file, then this
-	 * member may be NULL. Only access it through get_commit_tree()
-	 * or get_commit_tree_oid().
-	 */
-	struct tree *maybe_tree;
 	uint32_t graph_pos;
 };
 
diff --git a/merge-recursive.c b/merge-recursive.c
index 8de6e36527..a8b5f9ca39 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -23,6 +23,7 @@
 #include "merge-recursive.h"
 #include "dir.h"
 #include "submodule.h"
+#include "commit-graph.h"
 
 struct path_hashmap_entry {
 	struct hashmap_entry e;
@@ -101,7 +102,7 @@ static struct commit *make_virtual_commit(struct tree *tree, const char *comment
 	struct commit *commit = alloc_commit_node();
 
 	set_merge_remote_desc(commit, comment, (struct object *)commit);
-	commit->maybe_tree = tree;
+	set_maybe_tree(commit, tree);
 	commit->object.parsed = 1;
 	return commit;
 }
-- 8< --
Previous: Junio C HamanoNext: Duy Nguyen
Message 46 of 82 in “Die commit->util, die!”
  1. 00/12 Die commit->util, die!Nguyễn Thái Ngọc Duy, May 12, 2018
  2. 02/12 describe: use commit-slab for commit names instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  3. 05/12 sequencer.c: use commit-slab to associate todo items to commitsNguyễn Thái Ngọc Duy, May 12, 2018
  4. Jeff KingMay 12, 2018
  5. 04/12 sequencer.c: use commit-slab to mark seen commitsNguyễn Thái Ngọc Duy, May 12, 2018
  6. Jeff KingMay 12, 2018
  7. Junio C HamanoMay 12, 2018
  8. Duy NguyenMay 12, 2018
  9. 06/12 revision.c: use commit-slab for show_sourceNguyễn Thái Ngọc Duy, May 12, 2018
  10. Jeff KingMay 12, 2018
  11. Junio C HamanoMay 12, 2018
  12. Duy NguyenMay 12, 2018
  13. Jeff KingMay 12, 2018
  14. 10/12 log: use commit-slab in prepare_bases() instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  15. 12/12 commit.h: delete 'util' field in struct commitNguyễn Thái Ngọc Duy, May 12, 2018
  16. 11/12 merge: use commit-slab in merge remote desc instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  17. 09/12 show-branch: use commit-slab for commit-name instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  18. 01/12 blame: use commit-slab for blame suspects instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  19. Jeff KingMay 12, 2018
  20. Duy NguyenMay 12, 2018
  21. 07/12 bisect.c: use commit-slab for commit weight instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  22. Junio C HamanoMay 12, 2018
  23. 08/12 name-rev: use commit-slab for rev-name instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  24. 03/12 shallow.c: use commit-slab for commit depth instead of commit->utilNguyễn Thái Ngọc Duy, May 12, 2018
  25. Jeff KingMay 12, 2018
  26. Jeff KingMay 12, 2018
  27. Duy NguyenMay 12, 2018
  28. Jeff KingMay 12, 2018
  29. Duy NguyenMay 12, 2018
  30. Jeff KingMay 12, 2018
  31. Jakub NarebskiMay 12, 2018
  32. Duy NguyenMay 13, 2018
  33. 00/14 Die commit->util, die!Nguyễn Thái Ngọc Duy, May 13, 2018
  34. 01/14 commit-slab.h: code splitNguyễn Thái Ngọc Duy, May 13, 2018
  35. Junio C HamanoMay 13, 2018
  36. 02/14 commit-slab: support shared commit-slabNguyễn Thái Ngọc Duy, May 13, 2018
  37. Junio C HamanoMay 13, 2018
  38. 03/14 blame: use commit-slab for blame suspects instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  39. Junio C HamanoMay 13, 2018
  40. 06/14 sequencer.c: use commit-slab to mark seen commitsNguyễn Thái Ngọc Duy, May 13, 2018
  41. Junio C HamanoMay 14, 2018
  42. 12/14 log: use commit-slab in prepare_bases() instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  43. 07/14 sequencer.c: use commit-slab to associate todo items to commitsNguyễn Thái Ngọc Duy, May 13, 2018
  44. 14/14 commit.h: delete 'util' field in struct commitNguyễn Thái Ngọc Duy, May 13, 2018
  45. Junio C HamanoMay 14, 2018
  46. Duy NguyenMay 14, 2018
  47. Duy NguyenMay 14, 2018
  48. Derrick StoleeMay 14, 2018
  49. Duy NguyenMay 19, 2018
  50. 08/14 revision.c: use commit-slab for show_sourceNguyễn Thái Ngọc Duy, May 13, 2018
  51. Junio C HamanoMay 14, 2018
  52. Junio C HamanoMay 14, 2018
  53. 13/14 merge: use commit-slab in merge remote desc instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  54. Junio C HamanoMay 18, 2018
  55. Ramsay JonesMay 18, 2018
  56. 11/14 show-branch: use commit-slab for commit-name instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  57. Junio C HamanoMay 14, 2018
  58. Duy NguyenMay 19, 2018
  59. 09/14 bisect.c: use commit-slab for commit weight instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  60. Junio C HamanoMay 14, 2018
  61. 05/14 shallow.c: use commit-slab for commit depth instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  62. 10/14 name-rev: use commit-slab for rev-name instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  63. 04/14 describe: use commit-slab for commit names instead of commit->utilNguyễn Thái Ngọc Duy, May 13, 2018
  64. Junio C HamanoMay 13, 2018
  65. 00/15 Die commit->util, die!Nguyễn Thái Ngọc Duy, May 19, 2018
  66. 01/15 commit-slab.h: code splitNguyễn Thái Ngọc Duy, May 19, 2018
  67. Junio C HamanoMay 21, 2018
  68. 02/15 commit-slab: support shared commit-slabNguyễn Thái Ngọc Duy, May 19, 2018
  69. 04/15 describe: use commit-slab for commit names instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  70. 08/15 revision.c: use commit-slab for show_sourceNguyễn Thái Ngọc Duy, May 19, 2018
  71. 09/15 bisect.c: use commit-slab for commit weight instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  72. 07/15 sequencer.c: use commit-slab to associate todo items to commitsNguyễn Thái Ngọc Duy, May 19, 2018
  73. 13/15 log: use commit-slab in prepare_bases() instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  74. 15/15 commit.h: delete 'util' field in struct commitNguyễn Thái Ngọc Duy, May 19, 2018
  75. 14/15 merge: use commit-slab in merge remote desc instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  76. 06/15 sequencer.c: use commit-slab to mark seen commitsNguyễn Thái Ngọc Duy, May 19, 2018
  77. 12/15 show-branch: note about its object flags usageNguyễn Thái Ngọc Duy, May 19, 2018
  78. 11/15 show-branch: use commit-slab for commit-name instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  79. 10/15 name-rev: use commit-slab for rev-name instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  80. 03/15 blame: use commit-slab for blame suspects instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  81. 05/15 shallow.c: use commit-slab for commit depth instead of commit->utilNguyễn Thái Ngọc Duy, May 19, 2018
  82. Stefan BellerMay 22, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.