Re: [COMMITTERS] pgsql: Speedup pgstat_report_activity by movingmb-aware truncation to - Mailing list pgsql-committers

From Andres Freund
Subject Re: [COMMITTERS] pgsql: Speedup pgstat_report_activity by movingmb-aware truncation to
Date
Msg-id 20170919210228.wfo2q6qi3dwunhaf@alap3.anarazel.de
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Speedup pgstat_report_activity by moving mb-aware truncation to  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [COMMITTERS] pgsql: Speedup pgstat_report_activity by moving mb-aware truncation to  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-committers
On 2017-09-19 16:53:09 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.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


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

pgsql-committers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [COMMITTERS] pgsql: Make new crash restart test a bit morerobust.
Next
From: Tom Lane
Date:
Subject: Re: [COMMITTERS] pgsql: Make new crash restart test a bit more robust.