Re: appendStringInfo vs appendStringInfoString

On Sat, Sep 28, 2013 at 11:11 PM, Heikki Linnakangas <
hlinnakangas(at)vmware(dot)com> wrote:

> On 28.09.2013 12:44, David Rowley wrote:
>
>> The macro for test 4 was as follows:
>> #define appendStringInfoStringConst(**buf, s) appendBinaryStringInfo(buf,
>> (s), sizeof(s)-1)
>>
>
> If that makes any difference in practice, I wonder if we should just do:
>
> #define appendStringInfoString(buf, s) appendBinaryStringInfo(buf, (s),
> strlen(s))
>
> With a compiler worth its salt, the strlen(s) will be optimized into a
> constant, if s is a constant. If it's not a constant, we'll just end up
> calling strlen(), like appendStringInfoString would anyway. That would turn
> a single function call into two in all of the non-constant callsites,
> though, bloating the code, so it might not be a win overall.
>
>
I'm just looking at this again wondering how much impact changing
appendStringInfoString into a macro would have.

If I search the entire source for the regular
expression appendStringInfoString\(.*\,\s["].*["].*
which I think should match any usage with a string constant like
appendStringInfoString(buf, "some text here"); but perhaps may miss
instances that spread over more than 1 line. I'm getting 611 matches.

If I search for appendStringInfoString\(.*\,\s\w.* which should get the
instances that are not appending string constants I get 161 matches.
So it looks like with the macro idea it would only add the extra function
call in around 161 places and it would speed up 611 cases.

Note that I did these checks on my patched version of the source, the
current head will have less matches of each.

I just checked how much the binary increased in size after making this
change.

Before changing the macro the binary was 4,473,856 bytes
After changing the macro the binary is 4,476,928 bytes.

Regards

David

> - Heikki
>