On Sat, Apr 4, 2015 at 9:11 PM, Jeff King <peff@xxxxxxxx> wrote:
> 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 <peff@xxxxxxxx>
> ---
> 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.)
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html