Re: Replace some cstring_to_text to cstring_to_text_with_len

Em qui., 31 de ago. de 2023 às 12:12, Dagfinn Ilmari Mannsåker <
ilmari(at)ilmari(dot)org> escreveu:

> Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> writes:
>
> > Em qui., 31 de ago. de 2023 às 10:12, Dagfinn Ilmari Mannsåker <
> > ilmari(at)ilmari(dot)org> escreveu:
> >
> >> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >>
> >> > On 2023-08-31 Th 07:41, John Naylor wrote:
> >> >>
> >> >> On Thu, Aug 31, 2023 at 6:07 PM Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
> >> wrote:
> >> >> >
> >> >> > Em qui., 31 de ago. de 2023 às 00:22, Michael Paquier
> >> >> <michael(at)paquier(dot)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