Re: Proposal: add new API to stringinfo

On Mon, Jan 06, 2025 at 10:57:23AM +0900, Tatsuo Ishii wrote:
>> I'm also curious if this change has much of a meaningful impact in the
>> size of the compiled binary. The macro idea isn't quite how I'd have
>> done it as it does mean passing an extra parameter for every current
>> callsite. If I were doing this, I'd just have created the new
>> functions in stringinfo.c and have the current functions call the new
>> external function and rely on the compiler inlining, basically, that's
>> the same as what makeStringInfo() does today with initStringInfo().
>>
>> I'd have done it like:
>>
>> StringInfo
>> makeStringInfo(void)
>> {
>> return makeStringInfoExt(STRINGINFO_DEFAULT_SIZE);
>> }
>>
>> Using current master, you can see from the following that
>> initStringInfo is inlined, despite being an external function. I've no
>> reason to doubt the compiler won't do the same for makeStringInfo()
>> and makeStringInfoExt().
>>
>> $ gcc src/common/stringinfo.c -I src/include -O2 -S | cat stringinfo.s
>> | grep makeStringInfo -A 10
> [snip]
>> Note, there is no "call initStringInfo"
>
> I have tried with this direction. See attached v2 patch. In the asm
> code I don't see "call makeStringInfoExt" as expected. But I see
> "call initStringInfoExt". I assume this is an expected behavior.

Wouldn't that mean that initStringInfoExt() is not getting inlined? I
wonder if you need to create an inlined helper function that both the
original and the new "ext" versions use to stop gcc from emitting these
CALL instructions. For example:

static inline void
initStringInfoInternal(StringInfo str, int initsize)
{
...
}

static inline StringInfo
makeStringInfoInternal(int initsize)
{
StringInfo res = (StringInfo) palloc(sizeof(StringInfoData));

initStringInfoInternal(res, initsize);
return res;
}

StringInfo
makeStringInfo(void)
{
return makeStringInfoInternal(STRINGINFO_DEFAULT_SIZE);
}

StringInfo
makeStringInfoExt(int initsize)
{
return makeStringInfoInternal(initsize);
}

void
initStringInfo(StringInfo str)
{
initStringInfoInternal(str, STRINGINFO_DEFAULT_SIZE);
}

void
initStringInfoExt(StringInfo str, int initsize)
{
initStringInfoInternal(str, initsize);
}

That's admittedly quite verbose, but it ensures that all of these functions
only use the inlined versions of each other. If that works, it might be
worth doing something similar to resetStringInfo() (assuming it is not
already getting inlined).

--
nathan