On Sat, Sep 28, 2013 at 11:11 PM, Heikki Linnakangas < hlinnakan...@vmware.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 >