Re: [PATCH 2/2] log: do not shorten decoration names too early

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> The change may look something like this.  I do not think it would
>> make a difference to the get_cached_commit_buffer() codepath (when
>> we use the commit->buffer, we pretty much know we use that for all
>> commits), though.
>
> I'm not sure that's true. Most of the _users_ of the commit buffer will
> try to look in the cache, and if it's not there, do a one-off read.  But
> they don't attach the result to the commit; they throw it away. The
> reasoning is that we don't have a cached buffer because we are going to
> look at a lot of commits (i.e., save_commit_buffer is off).
>
> So basically anytime save_commit_buffer is off (e.g., in rev-list) we
> are expanding the slab unnecessarily, even though literally nobody will
> write to it.
> ...
> I think you need matching changes in unused_commit_buffer and
> free_commit_buffer. And detach_commit_buffer, too, I guess. Basically
> everywhere except set_commit_buffer would want to use the peek version.

Yeah, I didn't consider the "we are not using buffer but are still
calling into the slab code" case at all.

The other two slabs (indegree and author) in commit.c are used only
when needed, and when they are used they are fully populated anyway,
so I think this patch covers that file fully.

There are saved_parents slab used in revision.c and ref_bitmap slab
used in shallow.c; they may also need similar fixups to save memory.

 commit-slab.h | 27 ++++++++++++++++++++++++---
 commit.c      | 28 ++++++++++++++++++++--------
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/commit-slab.h b/commit-slab.h
index 375c9c7..3334ab2 100644
--- a/commit-slab.h
+++ b/commit-slab.h
@@ -15,7 +15,13 @@
  * - int *indegree_at(struct indegree *, struct commit *);
  *
  *   This function locates the data associated with the given commit in
- *   the indegree slab, and returns the pointer to it.
+ *   the indegree slab, and returns the pointer to it.  The location to
+ *   store the data is allocated as necessary.
+ *
+ * - int *indegree_peek(struct indegree *, struct commit *);
+ *
+ *   This function is similar to indegree_at(), but it will return NULL
+ *   until a call to indegree_at() was made for the commit.
  *
  * - void init_indegree(struct indegree *);
  *   void init_indegree_with_stride(struct indegree *, int);
@@ -80,8 +86,9 @@ static MAYBE_UNUSED void clear_ ##slabname(struct slabname *s)		\
 	s->slab = NULL;							\
 }									\
 									\
-static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s,	\
-				       const struct commit *c)		\
+static MAYBE_UNUSED elemtype *slabname## _at_peek(struct slabname *s,	\
+						  const struct commit *c, \
+						  int add_if_missing)   \
 {									\
 	int nth_slab, nth_slot;						\
 									\
@@ -90,6 +97,8 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s,	\
 									\
 	if (s->slab_count <= nth_slab) {				\
 		int i;							\
+		if (!add_if_missing)					\
+			return NULL;					\
 		s->slab = xrealloc(s->slab,				\
 				   (nth_slab + 1) * sizeof(*s->slab));	\
 		stat_ ##slabname## realloc++;				\
@@ -103,6 +112,18 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s,	\
 	return &s->slab[nth_slab][nth_slot * s->stride];				\
 }									\
 									\
+static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s,	\
+					     const struct commit *c)	\
+{									\
+	return slabname##_at_peek(s, c, 1);				\
+}									\
+									\
+static MAYBE_UNUSED elemtype *slabname## _peek(struct slabname *s,	\
+					     const struct commit *c)	\
+{									\
+	return slabname##_at_peek(s, c, 0);				\
+}									\
+									\
 static int stat_ ##slabname## realloc
 
 /*
diff --git a/commit.c b/commit.c
index 65179f9..4f2ce9f 100644
--- a/commit.c
+++ b/commit.c
@@ -244,7 +244,12 @@ void set_commit_buffer(struct commit *commit, void *buffer, unsigned long size)
 
 const void *get_cached_commit_buffer(const struct commit *commit, unsigned long *sizep)
 {
-	struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);
+	struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
+	if (!v) {
+		if (sizep)
+			*sizep = 0;
+		return NULL;
+	}
 	if (sizep)
 		*sizep = v->size;
 	return v->buffer;
@@ -271,24 +276,31 @@ const void *get_commit_buffer(const struct commit *commit, unsigned long *sizep)
 
 void unuse_commit_buffer(const struct commit *commit, const void *buffer)
 {
-	struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);
-	if (v->buffer != buffer)
+	struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
+	if (v && v->buffer != buffer)
 		free((void *)buffer);
 }
 
 void free_commit_buffer(struct commit *commit)
 {
-	struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);
-	free(v->buffer);
-	v->buffer = NULL;
-	v->size = 0;
+	struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
+	if (v) {
+		free(v->buffer);
+		v->buffer = NULL;
+		v->size = 0;
+	}
 }
 
 const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
 {
-	struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);
+	struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
 	void *ret;
 
+	if (!v) {
+		if (sizep)
+			*sizep = 0;
+		return NULL;
+	}
 	ret = v->buffer;
 	if (sizep)
 		*sizep = v->size;