On Thu, May 14, 2015 at 02:49:10PM -0700, Junio C Hamano wrote:
> > But if we do not have the "buffer" data cached for that commit (via
> > an earlier call to set_commit_buffer()), we don't have to enlarge
> > the slab, as we are not adding anything to the slab system with this
> > call.
>
> 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.
> diff --git a/commit.c b/commit.c
> index 65179f9..2d1901f 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;
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.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html