Re: [PATCHv2] pathspec: allow escaped query values

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 2, 2016, 23:01 UTC
Message-ID: <xmqq60trry9e.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kYL_47ptjK1S++Z=JUBOQtG1MJS=h0i=9f3fzRmbZDf-g@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> Thinking about efficiency, I have the believe that memmove can be faster
> than a `*src=*dst` thing we do ourselves as it may have access to specialized
> assembly instructions to move larger chunks of memory or such.

Show 9 quoted lines

> So I think ideally we would do a block copy between the escape characters
> (sketched as:)
>
>     last = input
>     while input not ended:
>         current = find next escape character in input
>         memcpy from input value in the range of last to current
>         last = current + 1
>     copy remaining parts if no further escape is found

That would be true _only_ when "find next escape and copy up to that byte" aka "scanning once with optimized strchr(), and copying once with optimized memmove()" is faster than "scanning once and copying" loop.

I was merely reacting to your use of memmove() in a very different loop, where if you unescape "a\b\c\defghijk", your memmove() would move "efghijk" many times.

>>         ret = xmalloc(strlen(value));
>
> xmallocz at least?

Yes. Also the handling of the terminating NUL may need to be updated. That is why I did not say "replace yours with this", but merely "along the lines of this" ;-)