Re: Replace some cstring_to_text to cstring_to_text_with_len

Поиск
Список
Период
Сортировка
От Ranier Vilela
Тема Re: Replace some cstring_to_text to cstring_to_text_with_len
Дата
Msg-id CAEudQAqnMyawtnaBJyqqdYoV-Mt7ccvYpSKT7kmyfTbS1kQTTw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Replace some cstring_to_text to cstring_to_text_with_len  (Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>)
Список pgsql-hackers
Em qui., 31 de ago. de 2023 às 12:12, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:

> Em qui., 31 de ago. de 2023 às 10:12, Dagfinn Ilmari Mannsåker <
> ilmari@ilmari.org> escreveu:
>
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>
>> > On 2023-08-31 Th 07:41, John Naylor wrote:
>> >>
>> >> On Thu, Aug 31, 2023 at 6:07 PM Ranier Vilela <ranier.vf@gmail.com>
>> wrote:
>> >> >
>> >> > Em qui., 31 de ago. de 2023 às 00:22, Michael Paquier
>> >> <michael@paquier.xyz> escreveu:
>> >> >>
>> >> >> On Wed, Aug 30, 2023 at 03:00:13PM -0300, Ranier Vilela wrote:
>> >> >> > cstring_to_text has a small overhead, because call strlen for
>> >> >> > pointer to char parameter.
>> >> >> >
>> >> >> > Is it worth the effort to avoid this, where do we know the size
>> >> of the
>> >> >> > parameter?
>> >> >>
>> >> >> Are there workloads where this matters?
>> >> >
>> >> > None, but note this change has the same spirit of 8b26769bc.
>> >>
>> >> - return cstring_to_text("");
>> >> + return cstring_to_text_with_len("", 0);
>> >>
>> >> This looks worse, so we'd better be getting something in return.
>> >
>> >
>> > I agree this is a bit ugly. I wonder if we'd be better off creating a
>> > function that returned an empty text value, so we'd just avoid
>> > converting the empty cstring altogether and say:
>> >
>> >   return empty_text();
>>
>> Or we could generalise it for any string literal (of which there are
>> slightly more¹ non-empty than empty in calls to
>> cstring_to_text(_with_len)):
>>
>> #define literal_to_text(str) cstring_to_text_with_len("" str "",
>> sizeof(str)-1)
>>
> I do not agree, I think this will get worse.

How exactly will it get worse?  It's exactly equivalent to
cstring_to_text_with_len("", 0), since sizeof() is a compile-time
construct, and the string concatenation makes it fail if the argument is
not a literal string.
I think that concatenation makes the strings twice bigger, doesn't it?
 

Whether we want an even-more-optimised version for an empty text value
is another matter, but I doubt it'd be worth it.  Another option would
be to make cstring_to_text(_with_len) static inline functions, which
lets the compiler eliminate the memcpy() call when len == 0.

In fact, after playing around a bit (https://godbolt.org/z/x51aYGadh),
it seems like GCC, Clang and MSVC all eliminate the strlen() and
memcpy() calls for cstring_to_text("") under -O2 if it's static inline.
In that case, it seems to me that would be good too.
Compilers removing memcpy would have the same as empty_text.
Without the burden of a new function and all its future maintenance.

best regards,
Ranier Vilela

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Jeff Davis
Дата:
Сообщение: Re: [17] CREATE SUBSCRIPTION ... SERVER
Следующее
От: Denis Smirnov
Дата:
Сообщение: Re: Use virtual tuple slot for Unique node