On Wed, Aug 18, 2021 at 6:51 AM ZheNing Hu <adlternative@xxxxxxxxx> wrote:
>
> Christian Couder <christian.couder@xxxxxxxxx> 于2021年8月18日周三 上午12:10写道：
> >
> > Hi,
> >
> > On Mon, Aug 16, 2021 at 4:00 PM ZheNing Hu <adlternative@xxxxxxxxx> wrote:
> > >
> > > Hi,
> > >
> > > In the implementation of %(raw) atom
> > > (bd0708c7 ref-filter: add %(raw) atom), we use xmemdupz()
> > > to copy the content of the object. But if we can reuse the content
> > > of the object?
> > >
> > > Since git cat-file --batch needs to use ref-filter
> > > as the backend, if the object buffer can be reused correctly here,
> > > we can save a lot of copies and improve its performance by about 6%.
> >
> > Yeah, that would be great.
> >
> > > Tracing back to the source, the object buffer is allocated from
> > > oid_object_info_extended(), but in parse_object_buffer() we may lose
> > > the ownership of the buffer (maybe the buffer is eaten), but I browsed the
> > > source code of for-each-ref.c or cat-file.c, and I don’t seem to find that the
> > > buffers which have been eaten are released by the program.
> > >
> > > So can we reuse it?
> > >
> > > This is what I want to do:
> > >
> > > diff --git a/ref-filter.c b/ref-filter.c
> > > index 93ce2a6ef2..1f6c1daabd 100644
> > > --- a/ref-filter.c
> > > +++ b/ref-filter.c
> > > @@ -1443,7 +1443,7 @@ static void grab_sub_body_contents(struct
> > > atom_value *val, int deref, struct exp
> > > unsigned long buf_size = data->size;
> > >
> > > if (atom->u.raw_data.option == RAW_BARE) {
> > > - v->s = xmemdupz(buf, buf_size);
> > > + v->s = buf;
> >
> > It seems to me that this could work only if 'buf' isn't freed. Have
> > you checked that? Did we leak 'buf' before this patch? Otherwise when
> > are we freeing it?
> >
> This is how I use gdb find out if this buffer have been freed:
I was asking about 'buf' before the patch.
Before the patch, we were doing:
v->s = xmemdupz(buf, buf_size);
which means that in v->s there is a copy of 'buf', not 'buf' itself.
So I was wondering if 'buf' was freed then, not the copy in in v->s.
