public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH] Precompute string lengths in PerformRadiusTransaction
4+ messages / 3 participants
[nested] [flat]

* [PATCH] Precompute string lengths in PerformRadiusTransaction
@ 2026-01-05 14:36 =?gb18030?B?emVuZ21hbg==?= <[email protected]>
  2026-01-05 14:51 ` Re: [PATCH] Precompute string lengths in PerformRadiusTransaction Aleksander Alekseev <[email protected]>
  2026-03-25 09:51 ` Re: [PATCH] Precompute string lengths in PerformRadiusTransaction John Naylor <[email protected]>
  0 siblings, 2 replies; 4+ messages in thread

From: =?gb18030?B?emVuZ21hbg==?= @ 2026-01-05 14:36 UTC (permalink / raw)
  To: =?gb18030?B?cGdzcWwtaGFja2Vycw==?= <[email protected]>

Hi, hackers

I noticed that the `PerformRadiusTransaction` function is invoked in a loop, 
and the function itself contains internal loops that repeatedly call strlen() on the 'passwd' and 'secret' strings. 
While the optimization gain may be minimal, I still think it’s worth making the change — 
we can precompute the lengths of these two strings at the start of the function to eliminate redundant strlen() calls.

--
Regards,
Man Zeng
www.openhalo.org

Attachments:

  [application/octet-stream] 0001-Precompute-string-lengths-in-PerformRadiusTransactio.patch (3.9K, 2-0001-Precompute-string-lengths-in-PerformRadiusTransactio.patch)
  download | inline diff:
From 98cc293f9050b3134dd3308f3efac1d3f4c13398 Mon Sep 17 00:00:00 2001
From: Man Zeng <[email protected]>
Date: Mon, 5 Jan 2026 22:04:56 +0800
Subject: [PATCH] Precompute string lengths in PerformRadiusTransaction

  Avoid repeated strlen() calls on 'passwd' and 'secret' strings by
  caching their lengths at function entry. This is a micro-optimization
  that reduces function call overhead in the password encryption and
  response verification loops.
---
 src/backend/libpq/auth.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 795bfed8d19..77a97224535 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2954,6 +2954,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	radius_packet *receivepacket = &radius_recv_pack;
 	void	   *radius_buffer = &radius_send_pack;
 	void	   *receive_buffer = &radius_recv_pack;
+	size_t		passwd_len = strlen(passwd);
+	size_t		secret_len = strlen(secret);
 	int32		service = pg_hton32(RADIUS_AUTHENTICATE_ONLY);
 	uint8	   *cryptvector;
 	int			encryptedpasswordlen;
@@ -3018,9 +3020,9 @@ 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));
+	encryptedpasswordlen = ((passwd_len + RADIUS_VECTOR_LENGTH - 1) / RADIUS_VECTOR_LENGTH) * RADIUS_VECTOR_LENGTH;
+	cryptvector = palloc(secret_len + RADIUS_VECTOR_LENGTH);
+	memcpy(cryptvector, secret, secret_len);
 
 	/* for the first iteration, we use the Request Authenticator vector */
 	md5trailer = packet->vector;
@@ -3028,7 +3030,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	{
 		const char *errstr = NULL;
 
-		memcpy(cryptvector + strlen(secret), md5trailer, RADIUS_VECTOR_LENGTH);
+		memcpy(cryptvector + secret_len, md5trailer, RADIUS_VECTOR_LENGTH);
 
 		/*
 		 * .. and for subsequent iterations the result of the previous XOR
@@ -3036,7 +3038,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		 */
 		md5trailer = encryptedpassword + i;
 
-		if (!pg_md5_binary(cryptvector, strlen(secret) + RADIUS_VECTOR_LENGTH,
+		if (!pg_md5_binary(cryptvector, secret_len + RADIUS_VECTOR_LENGTH,
 						   encryptedpassword + i, &errstr))
 		{
 			ereport(LOG,
@@ -3049,7 +3051,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 < passwd_len)
 				encryptedpassword[j] = passwd[j] ^ encryptedpassword[j];
 			else
 				encryptedpassword[j] = '\0' ^ encryptedpassword[j];
@@ -3216,7 +3218,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 + secret_len);
 
 		memcpy(cryptvector, receivepacket, 4);	/* code+id+length */
 		memcpy(cryptvector + 4, packet->vector, RADIUS_VECTOR_LENGTH);	/* request
@@ -3227,10 +3229,10 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 			memcpy(cryptvector + RADIUS_HEADER_LENGTH,
 				   (char *) receive_buffer + RADIUS_HEADER_LENGTH,
 				   packetlength - RADIUS_HEADER_LENGTH);
-		memcpy(cryptvector + packetlength, secret, strlen(secret));
+		memcpy(cryptvector + packetlength, secret, secret_len);
 
 		if (!pg_md5_binary(cryptvector,
-						   packetlength + strlen(secret),
+						   packetlength + secret_len,
 						   encryptedpassword, &errstr))
 		{
 			ereport(LOG,
-- 
2.45.2



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: [PATCH] Precompute string lengths in PerformRadiusTransaction
  2026-01-05 14:36 [PATCH] Precompute string lengths in PerformRadiusTransaction =?gb18030?B?emVuZ21hbg==?= <[email protected]>
@ 2026-01-05 14:51 ` Aleksander Alekseev <[email protected]>
  1 sibling, 0 replies; 4+ messages in thread

From: Aleksander Alekseev @ 2026-01-05 14:51 UTC (permalink / raw)
  To: pgsql-hackers <[email protected]>; +Cc: zengman <[email protected]>

Hi,

> I noticed that the `PerformRadiusTransaction` function is invoked in a loop,
> and the function itself contains internal loops that repeatedly call strlen() on the 'passwd' and 'secret' strings.
> While the optimization gain may be minimal, I still think it’s worth making the change —
> we can precompute the lengths of these two strings at the start of the function to eliminate redundant strlen() calls.

Good find. We may also consider making both passwd_len and secret_len `const`.

-- 
Best regards,
Aleksander Alekseev






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: [PATCH] Precompute string lengths in PerformRadiusTransaction
  2026-01-05 14:36 [PATCH] Precompute string lengths in PerformRadiusTransaction =?gb18030?B?emVuZ21hbg==?= <[email protected]>
@ 2026-03-25 09:51 ` John Naylor <[email protected]>
  2026-03-25 10:58   ` Re: [PATCH] Precompute string lengths in PerformRadiusTransaction =?gb18030?B?emVuZ21hbg==?= <[email protected]>
  1 sibling, 1 reply; 4+ messages in thread

From: John Naylor @ 2026-03-25 09:51 UTC (permalink / raw)
  To: zengman <[email protected]>; +Cc: pgsql-hackers <[email protected]>

On Mon, Jan 5, 2026 at 9:36 PM zengman <[email protected]> wrote:
> I noticed that the `PerformRadiusTransaction` function is invoked in a loop,
> and the function itself contains internal loops that repeatedly call strlen() on the 'passwd' and 'secret' strings.
> While the optimization gain may be minimal, I still think it’s worth making the change —
> we can precompute the lengths of these two strings at the start of the function to eliminate redundant strlen() calls.

Shortly after you posted this, it was decided that we're going to
remove this method entirely:

https://wiki.postgresql.org/wiki/RADIUS

-- 
John Naylor
Amazon Web Services





^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: [PATCH] Precompute string lengths in PerformRadiusTransaction
  2026-01-05 14:36 [PATCH] Precompute string lengths in PerformRadiusTransaction =?gb18030?B?emVuZ21hbg==?= <[email protected]>
  2026-03-25 09:51 ` Re: [PATCH] Precompute string lengths in PerformRadiusTransaction John Naylor <[email protected]>
@ 2026-03-25 10:58   ` =?gb18030?B?emVuZ21hbg==?= <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: =?gb18030?B?emVuZ21hbg==?= @ 2026-03-25 10:58 UTC (permalink / raw)
  To: =?gb18030?B?Sm9obiBOYXlsb3I=?= <[email protected]>; +Cc: =?gb18030?B?cGdzcWwtaGFja2Vycw==?= <[email protected]>

> > I noticed that the `PerformRadiusTransaction` function is invoked in a loop,
> > and the function itself contains internal loops that repeatedly call strlen() on the 'passwd' and 'secret' strings.
> > While the optimization gain may be minimal, I still think it’s worth making the change —
> > we can precompute the lengths of these two strings at the start of the function to eliminate redundant strlen() calls.
> 
> Shortly after you posted this, it was decided that we're going to
> remove this method entirely:
> 
> https://wiki.postgresql.org/wiki/RADIUS
Hi,

Okay, thank you very much for reminding me. I'll retract this patch.

--
regards,
Man Zeng

^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2026-03-25 10:58 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-05 14:36 [PATCH] Precompute string lengths in PerformRadiusTransaction =?gb18030?B?emVuZ21hbg==?= <[email protected]>
2026-01-05 14:51 ` Aleksander Alekseev <[email protected]>
2026-03-25 09:51 ` John Naylor <[email protected]>
2026-03-25 10:58   ` =?gb18030?B?emVuZ21hbg==?= <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox