Micro-optimizations to avoid some strlen calls.

Started by Ranier Vilelaover 4 years ago6 messages
#1Ranier Vilela
ranier.vf@gmail.com
1 attachment(s)

Hi,

There are some places, where strlen can have an overhead.
This patch tries to fix this.

Pass check-world at linux ubuntu (20.04) 64 bits.

regards,
Ranier Vilela

Attachments:

micro-optmization-avoid-strlen.patchtext/x-patch; charset=US-ASCII; name=micro-optmization-avoid-strlen.patchDownload
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index c14ca27c5e..2036072990 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2443,11 +2443,11 @@ ChooseIndexColumnNames(List *indexElems)
 			if (lc2 == NULL)
 				break;			/* found nonconflicting name */
 
-			sprintf(nbuf, "%d", i);
+			nlen = sprintf(nbuf, "%d", i);
 
 			/* Ensure generated names are shorter than NAMEDATALEN */
 			nlen = pg_mbcliplen(origname, strlen(origname),
-								NAMEDATALEN - 1 - strlen(nbuf));
+								NAMEDATALEN - 1 - nlen);
 			memcpy(buf, origname, nlen);
 			strcpy(buf + nlen, nbuf);
 			curname = buf;
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 8cc23ef7fb..4619f3887e 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -3014,6 +3014,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	uint8		encryptedpassword[RADIUS_MAX_PASSWORD_LENGTH];
 	uint8	   *md5trailer;
 	int			packetlength;
+	size_t      secretlen;
+	size_t		passwdlen;
 	pgsocket	sock;
 
 #ifdef HAVE_IPV6
@@ -3077,15 +3079,17 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	 * and then: e[i] = p[i] XOR MD5(secret + e[i-1]) for the following ones
 	 * (if necessary)
 	 */
-	encryptedpasswordlen = ((strlen(passwd) + RADIUS_VECTOR_LENGTH - 1) / RADIUS_VECTOR_LENGTH) * RADIUS_VECTOR_LENGTH;
-	cryptvector = palloc(strlen(secret) + RADIUS_VECTOR_LENGTH);
-	memcpy(cryptvector, secret, strlen(secret));
+	secretlen = strlen(secret);
+	passwdlen = strlen(passwd);
+	encryptedpasswordlen = ((passwdlen + RADIUS_VECTOR_LENGTH - 1) / RADIUS_VECTOR_LENGTH) * RADIUS_VECTOR_LENGTH;
+	cryptvector = palloc(secretlen + RADIUS_VECTOR_LENGTH);
+	memcpy(cryptvector, secret, secretlen);
 
 	/* for the first iteration, we use the Request Authenticator vector */
 	md5trailer = packet->vector;
 	for (i = 0; i < encryptedpasswordlen; i += RADIUS_VECTOR_LENGTH)
 	{
-		memcpy(cryptvector + strlen(secret), md5trailer, RADIUS_VECTOR_LENGTH);
+		memcpy(cryptvector + secretlen, md5trailer, RADIUS_VECTOR_LENGTH);
 
 		/*
 		 * .. and for subsequent iterations the result of the previous XOR
@@ -3093,7 +3097,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		 */
 		md5trailer = encryptedpassword + i;
 
-		if (!pg_md5_binary(cryptvector, strlen(secret) + RADIUS_VECTOR_LENGTH, encryptedpassword + i))
+		if (!pg_md5_binary(cryptvector, secretlen + RADIUS_VECTOR_LENGTH, encryptedpassword + i))
 		{
 			ereport(LOG,
 					(errmsg("could not perform MD5 encryption of password")));
@@ -3104,7 +3108,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 
 		for (j = i; j < i + RADIUS_VECTOR_LENGTH; j++)
 		{
-			if (j < strlen(passwd))
+			if (j < passwdlen)
 				encryptedpassword[j] = passwd[j] ^ encryptedpassword[j];
 			else
 				encryptedpassword[j] = '\0' ^ encryptedpassword[j];
@@ -3286,7 +3290,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		 * Verify the response authenticator, which is calculated as
 		 * MD5(Code+ID+Length+RequestAuthenticator+Attributes+Secret)
 		 */
-		cryptvector = palloc(packetlength + strlen(secret));
+		cryptvector = palloc(packetlength + secretlen);
 
 		memcpy(cryptvector, receivepacket, 4);	/* code+id+length */
 		memcpy(cryptvector + 4, packet->vector, RADIUS_VECTOR_LENGTH);	/* request
@@ -3295,10 +3299,10 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		if (packetlength > RADIUS_HEADER_LENGTH)	/* there may be no
 													 * attributes at all */
 			memcpy(cryptvector + RADIUS_HEADER_LENGTH, receive_buffer + RADIUS_HEADER_LENGTH, packetlength - RADIUS_HEADER_LENGTH);
-		memcpy(cryptvector + packetlength, secret, strlen(secret));
+		memcpy(cryptvector + packetlength, secret, secretlen);
 
 		if (!pg_md5_binary(cryptvector,
-						   packetlength + strlen(secret),
+						   packetlength + secretlen,
 						   encryptedpassword))
 		{
 			ereport(LOG,
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 122c2b05bd..7c8069cbe1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4277,10 +4277,11 @@ static void
 report_fork_failure_to_client(Port *port, int errnum)
 {
 	char		buffer[1000];
+	size_t		buflen;
 	int			rc;
 
 	/* Format the error message packet (always V2 protocol) */
-	snprintf(buffer, sizeof(buffer), "E%s%s\n",
+	buflen = snprintf(buffer, sizeof(buffer), "E%s%s\n",
 			 _("could not fork new process for connection: "),
 			 strerror(errnum));
 
@@ -4291,7 +4292,7 @@ report_fork_failure_to_client(Port *port, int errnum)
 	/* We'll retry after EINTR, but ignore all other failures */
 	do
 	{
-		rc = send(port->sock, buffer, strlen(buffer) + 1, 0);
+		rc = send(port->sock, buffer, buflen + 1, 0);
 	} while (rc < 0 && errno == EINTR);
 }
 
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index e09108d0ec..9b11030890 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -953,10 +953,11 @@ static void
 send_int8_string(StringInfoData *buf, int64 intval)
 {
 	char		is[32];
+	size_t		islen;
 
-	sprintf(is, INT64_FORMAT, intval);
-	pq_sendint32(buf, strlen(is));
-	pq_sendbytes(buf, is, strlen(is));
+	islen = sprintf(is, INT64_FORMAT, intval);
+	pq_sendint32(buf, islen);
+	pq_sendbytes(buf, is, islen);
 }
 
 static void
diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c
index 40c758d789..8caff217ed 100644
--- a/src/backend/storage/file/reinit.c
+++ b/src/backend/storage/file/reinit.c
@@ -268,6 +268,8 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
 	 */
 	if ((op & UNLOGGED_RELATION_INIT) != 0)
 	{
+		size_t namelen = strlen(forkNames[INIT_FORKNUM]);
+
 		/* Scan the directory. */
 		dbspace_dir = AllocateDir(dbspacedirname);
 		while ((de = ReadDir(dbspace_dir, dbspacedirname)) != NULL)
@@ -296,7 +298,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
 			oidbuf[oidchars] = '\0';
 			snprintf(dstpath, sizeof(dstpath), "%s/%s%s",
 					 dbspacedirname, oidbuf, de->d_name + oidchars + 1 +
-					 strlen(forkNames[INIT_FORKNUM]));
+					 namelen);
 
 			/* OK, we're ready to perform the actual copy. */
 			elog(DEBUG2, "copying %s to %s", srcpath, dstpath);
@@ -334,7 +336,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
 			oidbuf[oidchars] = '\0';
 			snprintf(mainpath, sizeof(mainpath), "%s/%s%s",
 					 dbspacedirname, oidbuf, de->d_name + oidchars + 1 +
-					 strlen(forkNames[INIT_FORKNUM]));
+					 namelen);
 
 			fsync_fname(mainpath, false);
 		}
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index d093ce8038..686fdc550b 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -313,6 +313,7 @@ cash_out(PG_FUNCTION_ARGS)
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
+	size_t		len;
 	int			digit_pos;
 	int			points,
 				mon_group;
@@ -378,6 +379,7 @@ cash_out(PG_FUNCTION_ARGS)
 	 * current digit position, with zero as the digit just left of the decimal
 	 * point, increasing to the right.
 	 */
+	len = strlen(ssymbol);
 	digit_pos = points;
 	do
 	{
@@ -389,8 +391,8 @@ cash_out(PG_FUNCTION_ARGS)
 		else if (digit_pos < 0 && (digit_pos % mon_group) == 0)
 		{
 			/* insert thousands sep, but only to left of radix point */
-			bufptr -= strlen(ssymbol);
-			memcpy(bufptr, ssymbol, strlen(ssymbol));
+			bufptr -= len;
+			memcpy(bufptr, ssymbol, len);
 		}
 
 		*(--bufptr) = ((uint64) value % 10) + '0';
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a2e0f8de7e..7ab9b9d67b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11163,6 +11163,7 @@ GUCArrayDelete(ArrayType *array, const char *name)
 {
 	struct config_generic *record;
 	ArrayType  *newarray;
+	size_t      namelen;
 	int			i;
 	int			index;
 
@@ -11182,6 +11183,7 @@ GUCArrayDelete(ArrayType *array, const char *name)
 
 	newarray = NULL;
 	index = 1;
+	namelen = strlen(name);
 
 	for (i = 1; i <= ARR_DIMS(array)[0]; i++)
 	{
@@ -11200,8 +11202,8 @@ GUCArrayDelete(ArrayType *array, const char *name)
 		val = TextDatumGetCString(d);
 
 		/* ignore entry if it's what we want to delete */
-		if (strncmp(val, name, strlen(name)) == 0
-			&& val[strlen(name)] == '=')
+		if (strncmp(val, name, namelen) == 0
+			&& val[namelen] == '=')
 			continue;
 
 		/* else add it to the output array */
@@ -12098,15 +12100,16 @@ assign_pgstat_temp_directory(const char *newval, void *extra)
 	char	   *dname;
 	char	   *tname;
 	char	   *fname;
+	size_t		newvallen = strlen(newval);
 
 	/* directory */
-	dname = guc_malloc(ERROR, strlen(newval) + 1);	/* runtime dir */
+	dname = guc_malloc(ERROR, newvallen + 1);	/* runtime dir */
 	sprintf(dname, "%s", newval);
 
 	/* global stats */
-	tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */
+	tname = guc_malloc(ERROR, newvallen + 12); /* /global.tmp */
 	sprintf(tname, "%s/global.tmp", newval);
-	fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */
+	fname = guc_malloc(ERROR, newvallen + 13); /* /global.stat */
 	sprintf(fname, "%s/global.stat", newval);
 
 	if (pgstat_stat_directory)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e950b41374..da76fc56e0 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -5153,6 +5153,7 @@ parseServiceFile(const char *serviceFile,
 				 PQExpBuffer errorMessage,
 				 bool *group_found)
 {
+	size_t		servicelen;
 	int			result = 0,
 				linenr = 0,
 				i;
@@ -5170,13 +5171,15 @@ parseServiceFile(const char *serviceFile,
 		return 1;
 	}
 
+	servicelen = strlen(service);
 	while ((line = fgets(buf, sizeof(buf), f)) != NULL)
 	{
 		int			len;
 
 		linenr++;
 
-		if (strlen(line) >= sizeof(buf) - 1)
+		len = strlen(line);
+		if (len >= sizeof(buf) - 1)
 		{
 			appendPQExpBuffer(errorMessage,
 							  libpq_gettext("line %d too long in service file \"%s\"\n"),
@@ -5187,7 +5190,6 @@ parseServiceFile(const char *serviceFile,
 		}
 
 		/* ignore whitespace at end of line, especially the newline */
-		len = strlen(line);
 		while (len > 0 && isspace((unsigned char) line[len - 1]))
 			line[--len] = '\0';
 
@@ -5208,8 +5210,8 @@ parseServiceFile(const char *serviceFile,
 				goto exit;
 			}
 
-			if (strncmp(line + 1, service, strlen(service)) == 0 &&
-				line[strlen(service) + 1] == ']')
+			if (strncmp(line + 1, service, servicelen) == 0 &&
+				line[servicelen + 1] == ']')
 				*group_found = true;
 			else
 				*group_found = false;
#2Michael Paquier
michael@paquier.xyz
In reply to: Ranier Vilela (#1)
Re: Micro-optimizations to avoid some strlen calls.

On Mon, Jul 19, 2021 at 07:48:55PM -0300, Ranier Vilela wrote:

There are some places, where strlen can have an overhead.
This patch tries to fix this.

Pass check-world at linux ubuntu (20.04) 64 bits.

Why does it matter? No code paths you are changing here are
performance-critical, meaning that such calls won't really show up
high in profiles.

I don't think there is anything to change here.
--
Michael

#3David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Paquier (#2)
Re: Micro-optimizations to avoid some strlen calls.

On Tue, Jul 20, 2021 at 5:28 PM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Jul 19, 2021 at 07:48:55PM -0300, Ranier Vilela wrote:

There are some places, where strlen can have an overhead.
This patch tries to fix this.

Pass check-world at linux ubuntu (20.04) 64 bits.

Why does it matter? No code paths you are changing here are
performance-critical, meaning that such calls won't really show up
high in profiles.

I don't think there is anything to change here.

Agreed. To borrow from a nearby email of a similar nature (PGConn
information retrieval IIRC) - it is not generally a benefit to avoid
function call access to data multiple times in a block by substituting in a
saved local variable. The function call tends to be more readable then
having yet one more unimportant name to keep in short-term memory. As much
code already conforms to that the status quo is a preferred state unless
there is a demonstrable performance gain to be had. The readability, and
lack of churn, is otherwise more important.

David J.

#4David Rowley
dgrowleyml@gmail.com
In reply to: Ranier Vilela (#1)
Re: Micro-optimizations to avoid some strlen calls.

On Tue, 20 Jul 2021 at 10:49, Ranier Vilela <ranier.vf@gmail.com> wrote:

There are some places, where strlen can have an overhead.
This patch tries to fix this.

I'm with Michael and David on this.

I don't really feel like doing;

- snprintf(buffer, sizeof(buffer), "E%s%s\n",
+ buflen = snprintf(buffer, sizeof(buffer), "E%s%s\n",
  _("could not fork new process for connection: "),

is a good idea. I'm unsure if you're either not aware of the value
that snprintf() returns or just happen to think an overflow is
unlikely enough because you're convinced that 1000 chars are always
enough to fit this translatable string. I'd say if we were 100%
certain of that then it might as well become sprintf() instead.
However, I imagine you'll struggle to get people to side with you that
taking this overflow risk would be worthwhile given your lack of any
evidence that anything actually has become meaningfully faster as a
result of any of these changes.

David

#5Ranier Vilela
ranier.vf@gmail.com
In reply to: David Rowley (#4)
Re: Micro-optimizations to avoid some strlen calls.

Em qua., 21 de jul. de 2021 às 07:44, David Rowley <dgrowleyml@gmail.com>
escreveu:

On Tue, 20 Jul 2021 at 10:49, Ranier Vilela <ranier.vf@gmail.com> wrote:

There are some places, where strlen can have an overhead.
This patch tries to fix this.

I'm with Michael and David on this.

I don't really feel like doing;

- snprintf(buffer, sizeof(buffer), "E%s%s\n",
+ buflen = snprintf(buffer, sizeof(buffer), "E%s%s\n",
_("could not fork new process for connection: "),

is a good idea. I'm unsure if you're either not aware of the value
that snprintf() returns or just happen to think an overflow is
unlikely enough because you're convinced that 1000 chars are always
enough to fit this translatable string. I'd say if we were 100%
certain of that then it might as well become sprintf() instead.
However, I imagine you'll struggle to get people to side with you that
taking this overflow risk would be worthwhile given your lack of any
evidence that anything actually has become meaningfully faster as a
result of any of these changes.

I got your point.
Really getting only the result of snprintf is a bad idea.
In this case, the right way would be:

snprintf(buffer, sizeof(buffer), "E%s%s\n",
_("could not fork new process for connection: "),
buflen = strlen(buffer);

Thus doesn't have to recount buffer over, if rc fails.
Thanks for the tip about snprintf, even though it's not the intention.
This is what I call a bad interface.

regards,
Ranier Vilela

Show quoted text
#6Ranier Vilela
ranier.vf@gmail.com
In reply to: Ranier Vilela (#5)
1 attachment(s)
Re: Micro-optimizations to avoid some strlen calls.

Em qua., 21 de jul. de 2021 às 09:28, Ranier Vilela <ranier.vf@gmail.com>
escreveu:

Em qua., 21 de jul. de 2021 às 07:44, David Rowley <dgrowleyml@gmail.com>
escreveu:

On Tue, 20 Jul 2021 at 10:49, Ranier Vilela <ranier.vf@gmail.com> wrote:

There are some places, where strlen can have an overhead.
This patch tries to fix this.

I'm with Michael and David on this.

I don't really feel like doing;

- snprintf(buffer, sizeof(buffer), "E%s%s\n",
+ buflen = snprintf(buffer, sizeof(buffer), "E%s%s\n",
_("could not fork new process for connection: "),

is a good idea. I'm unsure if you're either not aware of the value
that snprintf() returns or just happen to think an overflow is
unlikely enough because you're convinced that 1000 chars are always
enough to fit this translatable string. I'd say if we were 100%
certain of that then it might as well become sprintf() instead.
However, I imagine you'll struggle to get people to side with you that
taking this overflow risk would be worthwhile given your lack of any
evidence that anything actually has become meaningfully faster as a
result of any of these changes.

I got your point.
Really getting only the result of snprintf is a bad idea.
In this case, the right way would be:

snprintf(buffer, sizeof(buffer), "E%s%s\n",
_("could not fork new process for connection: "),
buflen = strlen(buffer);

Thus doesn't have to recount buffer over, if rc fails.
Thanks for the tip about snprintf, even though it's not the intention.
This is what I call a bad interface.

Here the v1 version, with fix to snprintf trap.

regards,
Ranier Vilela

Attachments:

v1-micro-optmization-avoid-strlen.patchapplication/octet-stream; name=v1-micro-optmization-avoid-strlen.patchDownload
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index c14ca27c5e..2036072990 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2443,11 +2443,11 @@ ChooseIndexColumnNames(List *indexElems)
 			if (lc2 == NULL)
 				break;			/* found nonconflicting name */
 
-			sprintf(nbuf, "%d", i);
+			nlen = sprintf(nbuf, "%d", i);
 
 			/* Ensure generated names are shorter than NAMEDATALEN */
 			nlen = pg_mbcliplen(origname, strlen(origname),
-								NAMEDATALEN - 1 - strlen(nbuf));
+								NAMEDATALEN - 1 - nlen);
 			memcpy(buf, origname, nlen);
 			strcpy(buf + nlen, nbuf);
 			curname = buf;
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 8cc23ef7fb..4619f3887e 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -3014,6 +3014,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	uint8		encryptedpassword[RADIUS_MAX_PASSWORD_LENGTH];
 	uint8	   *md5trailer;
 	int			packetlength;
+	size_t      secretlen;
+	size_t		passwdlen;
 	pgsocket	sock;
 
 #ifdef HAVE_IPV6
@@ -3077,15 +3079,17 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	 * and then: e[i] = p[i] XOR MD5(secret + e[i-1]) for the following ones
 	 * (if necessary)
 	 */
-	encryptedpasswordlen = ((strlen(passwd) + RADIUS_VECTOR_LENGTH - 1) / RADIUS_VECTOR_LENGTH) * RADIUS_VECTOR_LENGTH;
-	cryptvector = palloc(strlen(secret) + RADIUS_VECTOR_LENGTH);
-	memcpy(cryptvector, secret, strlen(secret));
+	secretlen = strlen(secret);
+	passwdlen = strlen(passwd);
+	encryptedpasswordlen = ((passwdlen + RADIUS_VECTOR_LENGTH - 1) / RADIUS_VECTOR_LENGTH) * RADIUS_VECTOR_LENGTH;
+	cryptvector = palloc(secretlen + RADIUS_VECTOR_LENGTH);
+	memcpy(cryptvector, secret, secretlen);
 
 	/* for the first iteration, we use the Request Authenticator vector */
 	md5trailer = packet->vector;
 	for (i = 0; i < encryptedpasswordlen; i += RADIUS_VECTOR_LENGTH)
 	{
-		memcpy(cryptvector + strlen(secret), md5trailer, RADIUS_VECTOR_LENGTH);
+		memcpy(cryptvector + secretlen, md5trailer, RADIUS_VECTOR_LENGTH);
 
 		/*
 		 * .. and for subsequent iterations the result of the previous XOR
@@ -3093,7 +3097,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		 */
 		md5trailer = encryptedpassword + i;
 
-		if (!pg_md5_binary(cryptvector, strlen(secret) + RADIUS_VECTOR_LENGTH, encryptedpassword + i))
+		if (!pg_md5_binary(cryptvector, secretlen + RADIUS_VECTOR_LENGTH, encryptedpassword + i))
 		{
 			ereport(LOG,
 					(errmsg("could not perform MD5 encryption of password")));
@@ -3104,7 +3108,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 
 		for (j = i; j < i + RADIUS_VECTOR_LENGTH; j++)
 		{
-			if (j < strlen(passwd))
+			if (j < passwdlen)
 				encryptedpassword[j] = passwd[j] ^ encryptedpassword[j];
 			else
 				encryptedpassword[j] = '\0' ^ encryptedpassword[j];
@@ -3286,7 +3290,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		 * Verify the response authenticator, which is calculated as
 		 * MD5(Code+ID+Length+RequestAuthenticator+Attributes+Secret)
 		 */
-		cryptvector = palloc(packetlength + strlen(secret));
+		cryptvector = palloc(packetlength + secretlen);
 
 		memcpy(cryptvector, receivepacket, 4);	/* code+id+length */
 		memcpy(cryptvector + 4, packet->vector, RADIUS_VECTOR_LENGTH);	/* request
@@ -3295,10 +3299,10 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		if (packetlength > RADIUS_HEADER_LENGTH)	/* there may be no
 													 * attributes at all */
 			memcpy(cryptvector + RADIUS_HEADER_LENGTH, receive_buffer + RADIUS_HEADER_LENGTH, packetlength - RADIUS_HEADER_LENGTH);
-		memcpy(cryptvector + packetlength, secret, strlen(secret));
+		memcpy(cryptvector + packetlength, secret, secretlen);
 
 		if (!pg_md5_binary(cryptvector,
-						   packetlength + strlen(secret),
+						   packetlength + secretlen,
 						   encryptedpassword))
 		{
 			ereport(LOG,
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 122c2b05bd..f79ebda39d 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4277,12 +4277,14 @@ static void
 report_fork_failure_to_client(Port *port, int errnum)
 {
 	char		buffer[1000];
+	size_t		buflen;
 	int			rc;
 
 	/* Format the error message packet (always V2 protocol) */
 	snprintf(buffer, sizeof(buffer), "E%s%s\n",
 			 _("could not fork new process for connection: "),
 			 strerror(errnum));
+	buflen = strlen(buffer);
 
 	/* Set port to non-blocking.  Don't do send() if this fails */
 	if (!pg_set_noblock(port->sock))
@@ -4291,7 +4293,7 @@ report_fork_failure_to_client(Port *port, int errnum)
 	/* We'll retry after EINTR, but ignore all other failures */
 	do
 	{
-		rc = send(port->sock, buffer, strlen(buffer) + 1, 0);
+		rc = send(port->sock, buffer, buflen + 1, 0);
 	} while (rc < 0 && errno == EINTR);
 }
 
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index e09108d0ec..9b11030890 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -953,10 +953,11 @@ static void
 send_int8_string(StringInfoData *buf, int64 intval)
 {
 	char		is[32];
+	size_t		islen;
 
-	sprintf(is, INT64_FORMAT, intval);
-	pq_sendint32(buf, strlen(is));
-	pq_sendbytes(buf, is, strlen(is));
+	islen = sprintf(is, INT64_FORMAT, intval);
+	pq_sendint32(buf, islen);
+	pq_sendbytes(buf, is, islen);
 }
 
 static void
diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c
index 40c758d789..8caff217ed 100644
--- a/src/backend/storage/file/reinit.c
+++ b/src/backend/storage/file/reinit.c
@@ -268,6 +268,8 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
 	 */
 	if ((op & UNLOGGED_RELATION_INIT) != 0)
 	{
+		size_t namelen = strlen(forkNames[INIT_FORKNUM]);
+
 		/* Scan the directory. */
 		dbspace_dir = AllocateDir(dbspacedirname);
 		while ((de = ReadDir(dbspace_dir, dbspacedirname)) != NULL)
@@ -296,7 +298,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
 			oidbuf[oidchars] = '\0';
 			snprintf(dstpath, sizeof(dstpath), "%s/%s%s",
 					 dbspacedirname, oidbuf, de->d_name + oidchars + 1 +
-					 strlen(forkNames[INIT_FORKNUM]));
+					 namelen);
 
 			/* OK, we're ready to perform the actual copy. */
 			elog(DEBUG2, "copying %s to %s", srcpath, dstpath);
@@ -334,7 +336,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
 			oidbuf[oidchars] = '\0';
 			snprintf(mainpath, sizeof(mainpath), "%s/%s%s",
 					 dbspacedirname, oidbuf, de->d_name + oidchars + 1 +
-					 strlen(forkNames[INIT_FORKNUM]));
+					 namelen);
 
 			fsync_fname(mainpath, false);
 		}
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index d093ce8038..686fdc550b 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -313,6 +313,7 @@ cash_out(PG_FUNCTION_ARGS)
 	char	   *result;
 	char		buf[128];
 	char	   *bufptr;
+	size_t		len;
 	int			digit_pos;
 	int			points,
 				mon_group;
@@ -378,6 +379,7 @@ cash_out(PG_FUNCTION_ARGS)
 	 * current digit position, with zero as the digit just left of the decimal
 	 * point, increasing to the right.
 	 */
+	len = strlen(ssymbol);
 	digit_pos = points;
 	do
 	{
@@ -389,8 +391,8 @@ cash_out(PG_FUNCTION_ARGS)
 		else if (digit_pos < 0 && (digit_pos % mon_group) == 0)
 		{
 			/* insert thousands sep, but only to left of radix point */
-			bufptr -= strlen(ssymbol);
-			memcpy(bufptr, ssymbol, strlen(ssymbol));
+			bufptr -= len;
+			memcpy(bufptr, ssymbol, len);
 		}
 
 		*(--bufptr) = ((uint64) value % 10) + '0';
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a2e0f8de7e..7ab9b9d67b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11163,6 +11163,7 @@ GUCArrayDelete(ArrayType *array, const char *name)
 {
 	struct config_generic *record;
 	ArrayType  *newarray;
+	size_t      namelen;
 	int			i;
 	int			index;
 
@@ -11182,6 +11183,7 @@ GUCArrayDelete(ArrayType *array, const char *name)
 
 	newarray = NULL;
 	index = 1;
+	namelen = strlen(name);
 
 	for (i = 1; i <= ARR_DIMS(array)[0]; i++)
 	{
@@ -11200,8 +11202,8 @@ GUCArrayDelete(ArrayType *array, const char *name)
 		val = TextDatumGetCString(d);
 
 		/* ignore entry if it's what we want to delete */
-		if (strncmp(val, name, strlen(name)) == 0
-			&& val[strlen(name)] == '=')
+		if (strncmp(val, name, namelen) == 0
+			&& val[namelen] == '=')
 			continue;
 
 		/* else add it to the output array */
@@ -12098,15 +12100,16 @@ assign_pgstat_temp_directory(const char *newval, void *extra)
 	char	   *dname;
 	char	   *tname;
 	char	   *fname;
+	size_t		newvallen = strlen(newval);
 
 	/* directory */
-	dname = guc_malloc(ERROR, strlen(newval) + 1);	/* runtime dir */
+	dname = guc_malloc(ERROR, newvallen + 1);	/* runtime dir */
 	sprintf(dname, "%s", newval);
 
 	/* global stats */
-	tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */
+	tname = guc_malloc(ERROR, newvallen + 12); /* /global.tmp */
 	sprintf(tname, "%s/global.tmp", newval);
-	fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */
+	fname = guc_malloc(ERROR, newvallen + 13); /* /global.stat */
 	sprintf(fname, "%s/global.stat", newval);
 
 	if (pgstat_stat_directory)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e950b41374..da76fc56e0 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -5153,6 +5153,7 @@ parseServiceFile(const char *serviceFile,
 				 PQExpBuffer errorMessage,
 				 bool *group_found)
 {
+	size_t		servicelen;
 	int			result = 0,
 				linenr = 0,
 				i;
@@ -5170,13 +5171,15 @@ parseServiceFile(const char *serviceFile,
 		return 1;
 	}
 
+	servicelen = strlen(service);
 	while ((line = fgets(buf, sizeof(buf), f)) != NULL)
 	{
 		int			len;
 
 		linenr++;
 
-		if (strlen(line) >= sizeof(buf) - 1)
+		len = strlen(line);
+		if (len >= sizeof(buf) - 1)
 		{
 			appendPQExpBuffer(errorMessage,
 							  libpq_gettext("line %d too long in service file \"%s\"\n"),
@@ -5187,7 +5190,6 @@ parseServiceFile(const char *serviceFile,
 		}
 
 		/* ignore whitespace at end of line, especially the newline */
-		len = strlen(line);
 		while (len > 0 && isspace((unsigned char) line[len - 1]))
 			line[--len] = '\0';
 
@@ -5208,8 +5210,8 @@ parseServiceFile(const char *serviceFile,
 				goto exit;
 			}
 
-			if (strncmp(line + 1, service, strlen(service)) == 0 &&
-				line[strlen(service) + 1] == ']')
+			if (strncmp(line + 1, service, servicelen) == 0 &&
+				line[servicelen + 1] == ']')
 				*group_found = true;
 			else
 				*group_found = false;