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

Re: [PATCH 2/3] strbuf.h API users: don't hardcode 8192, use STRBUF_HINT_SIZE

From
Jeff King <peff@peff.net>
Date
Jul 12, 2021, 17:58 UTC
Message-ID
<YOyC2gJ4PWCTepKn@coredump.intra.peff.net>
In-Reply-To
<xmqqo8bdda2j.fsf@gitster.g>
On Wed, Jul 07, 2021 at 03:37:56PM -0700, Junio C Hamano wrote:
Show 26 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> 
> > Change a couple of users of strbuf_init() that pass a hint of 8192 to
> > pass STRBUF_HINT_SIZE instead.
> >
> > Both of these hardcoded occurrences pre-date the use of the strbuf
> > API. See 5242bcbb638 (Use strbuf API in cache-tree.c, 2007-09-06) and
> > af6eb82262e (Use strbuf API in apply, blame, commit-tree and diff,
> > 2007-09-06).
> >
> > In both cases the exact choice of 8192 is rather arbitrary, e.g. for
> > commit buffers I think 1024 or 2048 would probably be a better
> > default (this commit message is getting this commit close to the
> > former, but I daresay it's already way above the average for git
> > commits).
> 
> Yes, they are arbitrary within the context of these callers.
> 
> I do not think using STRBUF_HINT_SIZE macro in them is the right
> thing to do at all, as there is no reason to think that the best
> value for the write chunk sizes in these codepath has any linkage to
> the best value for the read chunk sizes used by strbuf_read() at
> all.  When benchmarking reveals that the best default size for
> strbuf_read() is 16k, you'd update STRBUF_HINT_SIZE to 16k, but how
> do you tell that it also happens to be the best write buffer size
> for the cache-tree writeout codepath (answer: you don't)?

Being cc'd on this series, I feel compelled to respond with some review. But I'm in such agreement with what you said here (and downthread, and also in your response to patch 1) that I can only add a lame "me too". :)

-Peff
Previous: Randall S. BeckerNext: Ævar Arnfjörð Bjarmason
Message 9 of 12 in “strbuf.[ch]: add STRBUF_HINT_SIZE, don't hardcode 8192”
  1. 0/3 strbuf.[ch]: add STRBUF_HINT_SIZE, don't hardcode 8192Ævar Arnfjörð Bjarmason, Jul 7, 2021
  2. 1/3 strbuf.[ch]: add STRBUF_HINT_SIZE macro = 8192Ævar Arnfjörð Bjarmason, Jul 7, 2021
  3. Junio C HamanoJul 7, 2021
  4. 2/3 strbuf.h API users: don't hardcode 8192, use STRBUF_HINT_SIZEÆvar Arnfjörð Bjarmason, Jul 7, 2021
  5. Eric SunshineJul 7, 2021
  6. Junio C HamanoJul 7, 2021
  7. Junio C HamanoJul 7, 2021
  8. Randall S. BeckerJul 7, 2021
  9. Jeff KingJul 12, 2021
  10. 3/3 strbuf.[ch]: make strbuf_fread() take hint, not sizeÆvar Arnfjörð Bjarmason, Jul 7, 2021
  11. Han-Wen NienhuysJul 7, 2021
  12. Junio C HamanoJul 7, 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.