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

Re: [GSOC] [QUESTION] ref-filter: can %(raw) implement reuse oi.content?

From
ZheNing Hu <adlternative@gmail.com>
Date
Aug 18, 2021, 09:07 UTC
Message-ID
<CAOLTT8SjLPXT7ows-MZQLmDwzPN5pUrqj26+PHVbTevkBn3Tug@mail.gmail.com>
In-Reply-To
<CAP8UFD15vZ4B3dxPamaxqySZgLwffHU1Rx21bHRLY9zndjvAew@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> 于2021年8月18日周三 下午4:54写道:
Show 58 quoted lines
>
> On Wed, Aug 18, 2021 at 6:51 AM ZheNing Hu <adlternative@gmail.com> wrote:
> >
> > Christian Couder <christian.couder@gmail.com> 于2021年8月18日周三 上午12:10写道:
> > >
> > > Hi,
> > >
> > > On Mon, Aug 16, 2021 at 4:00 PM ZheNing Hu <adlternative@gmail.com> 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.
I think the 'buf' is not freed in ref-filter logic.
But one thing worth noting is:

parse_object_buffer() may take this buffer away, and store it in tree->buffer or use set_commit_buffer() to store it in commit_slab.

So in theory, as long as we don’t use parse_object_buffer(), this dynamically allocated memory can be used freely for us. If we use parse_object_buffer() and free the buffer, there seems to be a risk, but it does not appear so far.

-- ZheNing Hu

Previous: Christian CouderNext: ZheNing Hu
Message 6 of 14 in “[GSOC] [QUESTION] ref-filter: can %(raw) implement reuse oi.content?”
  1. ZheNing HuAug 16, 2021
  2. Fwd: [GSOC] [QUESTION] ref-filter: can %(raw) implement reuse oi.content?ZheNing Hu, Aug 17, 2021
  3. Christian CouderAug 17, 2021
  4. ZheNing HuAug 18, 2021
  5. Christian CouderAug 18, 2021
  6. ZheNing HuAug 18, 2021
  7. ZheNing HuAug 18, 2021
  8. ZheNing HuAug 19, 2021
  9. Christian CouderAug 20, 2021
  10. ZheNing HuAug 21, 2021
  11. Christian CouderAug 20, 2021
  12. ZheNing HuAug 21, 2021
  13. Christian CouderAug 24, 2021
  14. ZheNing HuAug 25, 2021

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.