Thread (8 messages) 8 messages, 2 authors, 2024-03-07

Re: [PATCH 1/2] mem-pool: add mem_pool_strfmt()

From: Jeff King <hidden>
Date: 2024-03-07 09:58:42

On Tue, Feb 27, 2024 at 06:58:26PM +0100, René Scharfe wrote:
quoted
quoted
It would not allow the shortcut of using the vast pool as a scratch
space to format the string with a single vsnprintf call in most cases.
Or am I missing something?
So here it sounds like you do care about some of the performance
aspects. So no, it would not allow that. You'd be using the vast pool of
heap memory provided by malloc(), and trusting that a call to malloc()
is not that expensive in practice. I don't know how true that is, or how
much it matters for this case.
In the specific use case we can look at three cases:

1. xstrfmt() [before 1c56fc2084]
2. size calculation + pre-size + strbuf_addf() [1c56fc2084]
3. mem_pool_strfmt() [this patch]

The performance of 2 and 3 is basically the same, 1 was worse.

xstrfmt() and strbuf_addf() both wrap strbuf_vaddf(), which uses
malloc() and vsnprintf().  My conclusion is that malloc() is fast
enough, but running vsnprintf() twice is slow (first time to determine
the allocation size, second time to actually build the string).  With a
memory pool we almost always only need to call it once per string, and
that's why I use it here.

The benefit of this patch (to me) is better code readability (no more
manual pre-sizing) without sacrificing performance.

The ability to clear (or UNLEAK) all these strings in one go is just a
bonus.
Ah, OK. I admit I did not read the series all that carefully. Mostly I
am just concerned about a world where there are parallel-universe
versions of every allocating function that takes a mem-pool. If it's
limited to this obvious string formatting variant that may not be too
bad, though.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help