Re: pgsql: Speedup pgstat_report_activity by moving mb-aware truncation to

On 2017-09-19 16:53:09 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2017-09-19 16:25:31 -0400, Tom Lane wrote:
> >> Looks like you're going to need to not depend on strnlen().
>
> > Yup noticed - we have pg_strnlen as a static function in
> > src/port/snprintf. I'm tempted to just make that public, rather than
> > open code the equivalent?
>
> No, we should not tie this to whether we're using port/snprintf.c.

Oh, I would have moved it to src/common/string.c, sorry for not
mentioning that.

> Personally, I don't see why this isn't coded more like
>
> int rawlen = strlen(activity);
>
> rawlen = Min(rawlen, pgstat_track_activity_query_size - 1);

Pure paranoia, but probably not justified paranoia. We take care that
there's always a trailing NULL byte, even under concurrent
modifications (which exist due to pgstat_get_backend_current_activity()).

I'll just change things so that the pnstrdup() happens first, and then
do a plain strlen() on that. Same paranoia, less cost.

> It seems likely to me that strlen() is going to be optimized a lot
> harder than strnlen() on most platforms, so that this way would win
> not only for strings up to pgstat_track_activity_query_size but for
> strings some way beyond that. Yes, for really really long queries
> the strnlen way would win, but that's optimizing for the wrong case IMO.

I wasn't thinking of optimizing this - and I've a hard time seing a case
where even the naive snprintf pg_strnlen(), or a plain strlen() would be
relevant performancewise.

Greetings,

Andres Freund