Re: [PATCH 4/6] strbuf: add an optimized 1-character strbuf_grow
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:22
On Sat, Apr 4, 2015 at 9:11 PM, Jeff King [off-list ref] wrote:
quoted hunk ↗ jump to hunk
We have to call strbuf_grow anytime we are going to add data to a strbuf. In most cases, it's a noop (since we grow the buffer aggressively), and the cost of the function call and size check is dwarfed by the actual buffer operation. For a tight loop of single-character additions, though, this overhead is noticeable. Furthermore, the single-character case is much easier to check; since the "extra" parameter is 1, we can do it without worrying about overflow. This patch adds a simple inline function for checking single-character growth. For the growth case, it just calls into the regular strbuf_grow(). This is redundant, as strbuf_grow will check again whether we need to grow. But it keeps our inline code simple, and most calls will not need to grow, so it's OK to treat this as a rare "slow path". We apply the new function to strbuf_getwholeline. [...] Signed-off-by: Jeff King <redacted> ---diff --git a/strbuf.c b/strbuf.c index af2bad4..2facd5f 100644 --- a/strbuf.c +++ b/strbuf.c@@ -445,7 +445,7 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term) strbuf_reset(sb); flockfile(fp); while ((ch = getc_unlocked(fp)) != EOF) { - strbuf_grow(sb, 1); + strbuf_grow_ch(sb);
strbuf_grow_ch() seems overly special-case. What about instead taking
advantage of inline strbuf_avail() to do something like this?
if (!strbuf_avail())
strbuf_grow(sb, 1);
(Minor tangent: The 1 is still slightly magical and potentially
confusing for someone who doesn't know that the buffer is grown
aggressively, so changing it to a larger number might make it more
obvious to the casual reader that the buffer is in fact not being
grown on every iteration.)
quoted hunk ↗ jump to hunk
sb->buf[sb->len++] = ch; if (ch == term) break;diff --git a/strbuf.h b/strbuf.h index 1883494..ef41151 100644 --- a/strbuf.h +++ b/strbuf.h@@ -137,6 +137,15 @@ static inline size_t strbuf_avail(const struct strbuf *sb) */ extern void strbuf_grow(struct strbuf *, size_t); +/* + * An optimized version of strbuf_grow() for a single character. + */ +static inline void strbuf_grow_ch(struct strbuf *sb) +{ + if (!sb->alloc || sb->alloc - 1 <= sb->len) + strbuf_grow(sb, 1); +} + /** * Set the length of the buffer to a given value. This function does *not* * allocate new memory, so you should not perform a `strbuf_setlen()` to a --2.4.0.rc0.363.gf9f328b