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

Re: [PATCH v3 1/4] replace: add --graft option

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 9, 2014, 16:43 UTC
Message-ID
<xmqq61kaf2zo.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140608120439.GA21827@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 30 quoted lines
> I think it would make sense to actually take this one step further,
> though, and move commit->buffer into the slab, as well. That has two
> advantages:
>
>   1. It further decreases the size of "struct commit" for callers who do
>      not use save_commit_buffer.
>
>   2. It ensures that no new callers crop up who set "commit->buffer" but
>      to not save the size in the slab (you can see in the patch below
>      that I had to modify builtin/blame.c, which (ab)uses
>      commit->buffer).
>
> It would be more disruptive to existing callers, but I think the end
> result would be pretty clean. The API would be something like:
>
>   /* attach buffer to commit */
>   set_commit_buffer(struct commit *, void *buf, unsigned long size);
>
>   /* get buffer, either from slab cache or by calling read_sha1_file */
>   void *get_commit_buffer(struct commit *, unsigned long *size);
>
>   /* free() an allocated buffer from above, noop for cached buffer */
>   void unused_commit_buffer(struct commit *, void *buf);
>
>   /* drop saved commit buffer to free memory */
>   void free_commit_buffer(struct commit *);
>
> The "get" function would serve the existing callers in pretty.c, as well
> as the one I'm adding elsewhere in show_signature. And it should work as
> a drop-in read_sha1_file/free replacement for you here.

This solution has the kind of niceness to make everybody (certainly including me) who has been involved in the code path should say "why didn't I think of that!" ;-)

Previous: Jeff KingNext: Junio C Hamano
Message 10 of 19 in “Add --graft option to git replace”
  1. 0/4 Add --graft option to git replaceChristian Couder, Jun 4, 2014
  2. 1/4 replace: add --graft optionChristian Couder, Jun 4, 2014
  3. Junio C HamanoJun 5, 2014
  4. Christian CouderJun 6, 2014
  5. Christian CouderJun 6, 2014
  6. Christian CouderJun 8, 2014
  7. Jeff KingJun 8, 2014
  8. Jeff KingJun 8, 2014
  9. Jeff KingJun 8, 2014
  10. Junio C HamanoJun 9, 2014
  11. Junio C HamanoJun 6, 2014
  12. 2/4 replace: add test for --graftChristian Couder, Jun 4, 2014
  13. 3/4 Documentation: replace: add --graft optionChristian Couder, Jun 4, 2014
  14. 4/4 contrib: add convert-grafts-to-replace-refs.shChristian Couder, Jun 4, 2014
  15. Junio C HamanoJun 5, 2014
  16. Christian CouderJun 6, 2014
  17. Christian CouderJun 29, 2014
  18. Junio C HamanoJun 30, 2014
  19. Christian CouderJun 30, 2014

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.