Re: [PATCH] read-cache: avoid memcpy in expand_name_field in index v4
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 18, 2013, 17:50 UTC
- Message-ID
- <7vboagoav9.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1363611482-1015-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> perf reports memcpy at the the 6th position [1] in "git status -uno" > using index v4, and strbuf_remove() in expand_name_field() accounts > for 25% of that. What we need here is a simple string cut and a > cheaper strbuf_setlen() should be enough.
While it is true that strbuf_remove(&sb, sb.len - trim, trim) is equivalent to strbuf_setlen(&sb, sb.len - trim), I wonder why we see any memcpy() in the first place.
strbuf_remove(&sb, sb.len - trim, trim) is turned into strbuf_splice(&sb, sb.len - trim, trim, NULL, 0) and then in turn it does these two:
memmove(sb.buf + (sb.len - trim) + 0,
sb.buf + sb.len, 0);
memcpy(sb.buf + (sb.len - trim), NULL, 0);both of which should be a no-op, no?
There also is this call that has the same "trim at the right end":
pretty.c: strbuf_remove(sb, sb->len - trimlen, trimlen);
It almost makes me suggest that it may be a better solution to make strbuf_remove() more intelligent about such a call pattern _if_ these empty memmove/memcpy are so expensive, perhaps like the attached. It could be that strbuf_splice() should be the one that ought to be more intelligent, but I'll leave it up to you to benchmark to find out where the best place to optimize is.
strbuf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/strbuf.c b/strbuf.c index 05d0693..12db700 100644 --- a/strbuf.c +++ b/strbuf.c @@ -179,7 +179,10 @@ void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len) void strbuf_remove(struct strbuf *sb, size_t pos, size_t len) { - strbuf_splice(sb, pos, len, NULL, 0); + if (pos + len == sb->len) + strbuf_setlen(sb, pos); + else + strbuf_splice(sb, pos, len, NULL, 0); } void strbuf_add(struct strbuf *sb, const void *data, size_t len)