Speedup and increase usability of set proc title functions

The setting of the process title could be seen on profiles of very
fast-to-execute queries.  In many locations where we call
set_ps_display() we pass along a string constant, the length of which is
known during compilation.  Here we effectively rename set_ps_display() to
set_ps_display_with_len() and then add a static inline function named
set_ps_display() which calls strlen() on the given string.  This allows
the compiler to optimize away the strlen() call when dealing with
call sites passing a string constant.  We can then also use memcpy()
instead of strlcpy() to copy the string into the destination buffer.
That's significantly faster than strlcpy's byte-at-a-time way of
copying.

Here we also take measures to improve some code which was adjusting the
process title to add a " waiting" suffix to it.  Call sites which require
this can now just call set_ps_display_suffix() to add or adjust the suffix
and call set_ps_display_remove_suffix() to remove it again.

Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAApHDvocBvvk-0gWNA2Gohe+sv9fMcv+fK_G+siBKJrgDG4O7g@mail.gmail.com

src/backend/replication/syncrep.c

@@ -148,8 +148,6 @@ static bool SyncRepQueueIsOrderedByLSN(int mode);
 void
 SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
 {
-	char	   *new_status = NULL;
-	const char *old_status;
 	int			mode;
 
 	/*
@@ -216,15 +214,10 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
 	/* Alter ps display to show waiting for sync rep. */
 	if (update_process_title)
 	{
-		int			len;
+		char		buffer[32];
 
-		old_status = get_ps_display(&len);
-		new_status = (char *) palloc(len + 32 + 1);
-		memcpy(new_status, old_status, len);
-		sprintf(new_status + len, " waiting for %X/%X",
-				LSN_FORMAT_ARGS(lsn));
-		set_ps_display(new_status);
-		new_status[len] = '\0'; /* truncate off " waiting ..." */
+		sprintf(buffer, "waiting for %X/%X", LSN_FORMAT_ARGS(lsn));
+		set_ps_display_suffix(buffer);
 	}
 
 	/*
@@ -322,12 +315,9 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
 	MyProc->syncRepState = SYNC_REP_NOT_WAITING;
 	MyProc->waitLSN = 0;
 
-	if (new_status)
-	{
-		/* Reset ps display */
-		set_ps_display(new_status);
-		pfree(new_status);
-	}
+	/* reset ps display to remove the suffix */
+	if (update_process_title)
+		set_ps_display_remove_suffix();
 }
 
 /*

src/backend/storage/buffer/bufmgr.c

@@ -4302,8 +4302,8 @@ void
 LockBufferForCleanup(Buffer buffer)
 {
 	BufferDesc *bufHdr;
-	char	   *new_status = NULL;
 	TimestampTz waitStart = 0;
+	bool		waiting = false;
 	bool		logged_recovery_conflict = false;
 
 	Assert(BufferIsPinned(buffer));
@@ -4350,11 +4350,11 @@ LockBufferForCleanup(Buffer buffer)
 									waitStart, GetCurrentTimestamp(),
 									NULL, false);
 
-			/* Report change to non-waiting status */
-			if (new_status)
+			if (waiting)
 			{
-				set_ps_display(new_status);
-				pfree(new_status);
+				/* reset ps display to remove the suffix if we added one */
+				set_ps_display_remove_suffix();
+				waiting = false;
 			}
 			return;
 		}
@@ -4374,18 +4374,11 @@ LockBufferForCleanup(Buffer buffer)
 		/* Wait to be signaled by UnpinBuffer() */
 		if (InHotStandby)
 		{
-			/* Report change to waiting status */
-			if (update_process_title && new_status == NULL)
+			if (!waiting)
 			{
-				const char *old_status;
-				int			len;
-
-				old_status = get_ps_display(&len);
-				new_status = (char *) palloc(len + 8 + 1);
-				memcpy(new_status, old_status, len);
-				strcpy(new_status + len, " waiting");
-				set_ps_display(new_status);
-				new_status[len] = '\0'; /* truncate off " waiting" */
+				/* adjust the process title to indicate that it's waiting */
+				set_ps_display_suffix("waiting");
+				waiting = true;
 			}
 
 			/*

src/backend/storage/ipc/standby.c

@@ -362,7 +362,7 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 									   bool report_waiting)
 {
 	TimestampTz waitStart = 0;
-	char	   *new_status = NULL;
+	bool		waiting = false;
 	bool		logged_recovery_conflict = false;
 
 	/* Fast exit, to avoid a kernel call if there's no work to be done. */
@@ -400,14 +400,14 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 					pg_usleep(5000L);
 			}
 
-			if (waitStart != 0 && (!logged_recovery_conflict || new_status == NULL))
+			if (waitStart != 0 && (!logged_recovery_conflict || !waiting))
 			{
 				TimestampTz now = 0;
 				bool		maybe_log_conflict;
 				bool		maybe_update_title;
 
 				maybe_log_conflict = (log_recovery_conflict_waits && !logged_recovery_conflict);
-				maybe_update_title = (update_process_title && new_status == NULL);
+				maybe_update_title = (update_process_title && !waiting);
 
 				/* Get the current timestamp if not report yet */
 				if (maybe_log_conflict || maybe_update_title)
@@ -420,15 +420,8 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 				if (maybe_update_title &&
 					TimestampDifferenceExceeds(waitStart, now, 500))
 				{
-					const char *old_status;
-					int			len;
-
-					old_status = get_ps_display(&len);
-					new_status = (char *) palloc(len + 8 + 1);
-					memcpy(new_status, old_status, len);
-					strcpy(new_status + len, " waiting");
-					set_ps_display(new_status);
-					new_status[len] = '\0'; /* truncate off " waiting" */
+					set_ps_display_suffix("waiting");
+					waiting = true;
 				}
 
 				/*
@@ -456,12 +449,10 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 		LogRecoveryConflict(reason, waitStart, GetCurrentTimestamp(),
 							NULL, false);
 
-	/* Reset ps display if we changed it */
-	if (new_status)
-	{
-		set_ps_display(new_status);
-		pfree(new_status);
-	}
+	/* reset ps display to remove the suffix if we added one */
+	if (waiting)
+		set_ps_display_remove_suffix();
+
 }
 
 /*

src/backend/storage/lmgr/lock.c

@@ -1810,24 +1810,12 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
 {
 	LOCKMETHODID lockmethodid = LOCALLOCK_LOCKMETHOD(*locallock);
 	LockMethod	lockMethodTable = LockMethods[lockmethodid];
-	char	   *volatile new_status = NULL;
 
 	LOCK_PRINT("WaitOnLock: sleeping on lock",
 			   locallock->lock, locallock->tag.mode);
 
-	/* Report change to waiting status */
-	if (update_process_title)
-	{
-		const char *old_status;
-		int			len;
-
-		old_status = get_ps_display(&len);
-		new_status = (char *) palloc(len + 8 + 1);
-		memcpy(new_status, old_status, len);
-		strcpy(new_status + len, " waiting");
-		set_ps_display(new_status);
-		new_status[len] = '\0'; /* truncate off " waiting" */
-	}
+	/* adjust the process title to indicate that it's waiting */
+	set_ps_display_suffix("waiting");
 
 	awaitedLock = locallock;
 	awaitedOwner = owner;
@@ -1874,12 +1862,8 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
 	{
 		/* In this path, awaitedLock remains set until LockErrorCleanup */
 
-		/* Report change to non-waiting status */
-		if (update_process_title)
-		{
-			set_ps_display(new_status);
-			pfree(new_status);
-		}
+		/* reset ps display to remove the suffix */
+		set_ps_display_remove_suffix();
 
 		/* and propagate the error */
 		PG_RE_THROW();
@@ -1888,12 +1872,8 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
 
 	awaitedLock = NULL;
 
-	/* Report change to non-waiting status */
-	if (update_process_title)
-	{
-		set_ps_display(new_status);
-		pfree(new_status);
-	}
+	/* reset ps display to remove the suffix */
+	set_ps_display_remove_suffix();
 
 	LOCK_PRINT("WaitOnLock: wakeup on lock",
 			   locallock->lock, locallock->tag.mode);

src/backend/tcop/postgres.c

@@ -1071,6 +1071,8 @@ exec_simple_query(const char *query_string)
 		Portal		portal;
 		DestReceiver *receiver;
 		int16		format;
+		const char *cmdtagname;
+		size_t		cmdtaglen;
 
 		pgstat_report_query_id(0, true);
 
@@ -1081,8 +1083,9 @@ exec_simple_query(const char *query_string)
 		 * destination.
 		 */
 		commandTag = CreateCommandTag(parsetree->stmt);
+		cmdtagname = GetCommandTagNameAndLen(commandTag, &cmdtaglen);
 
-		set_ps_display(GetCommandTagName(commandTag));
+		set_ps_display_with_len(cmdtagname, cmdtaglen);
 
 		BeginCommand(commandTag, dest);
 
@@ -2064,6 +2067,8 @@ exec_execute_message(const char *portal_name, long max_rows)
 	char		msec_str[32];
 	ParamsErrorCbData params_data;
 	ErrorContextCallback params_errcxt;
+	const char *cmdtagname;
+	size_t		cmdtaglen;
 
 	/* Adjust destination to tell printtup.c what to do */
 	dest = whereToSendOutput;
@@ -2110,7 +2115,9 @@ exec_execute_message(const char *portal_name, long max_rows)
 
 	pgstat_report_activity(STATE_RUNNING, sourceText);
 
-	set_ps_display(GetCommandTagName(portal->commandTag));
+	cmdtagname = GetCommandTagNameAndLen(portal->commandTag, &cmdtaglen);
+
+	set_ps_display_with_len(cmdtagname, cmdtaglen);
 
 	if (save_log_statement_stats)
 		ResetUsage();

src/backend/utils/misc/ps_status.c

@@ -86,6 +86,14 @@ static size_t ps_buffer_cur_len;	/* nominal strlen(ps_buffer) */
 
 static size_t ps_buffer_fixed_size; /* size of the constant prefix */
 
+/*
+ * Length of ps_buffer before the suffix was appeneded to the end, or 0 if we
+ * didn't set a suffix.
+ */
+static size_t ps_buffer_nosuffix_len;
+
+static void flush_ps_display(void);
+
 #endif							/* not PS_USE_NONE */
 
 /* save the original argv[] location here */
@@ -300,37 +308,158 @@ init_ps_display(const char *fixed_part)
 #endif							/* not PS_USE_NONE */
 }
 
-
-
-/*
- * Call this to update the ps status display to a fixed prefix plus an
- * indication of what you're currently doing passed in the argument.
- */
-void
-set_ps_display(const char *activity)
-{
 #ifndef PS_USE_NONE
+/*
+ * update_ps_display_precheck
+ *		Helper function to determine if updating the process title is
+ *		something that we need to do.
+ */
+static bool
+update_ps_display_precheck(void)
+{
 	/* update_process_title=off disables updates */
 	if (!update_process_title)
-		return;
+		return false;
 
 	/* no ps display for stand-alone backend */
 	if (!IsUnderPostmaster)
-		return;
+		return false;
 
 #ifdef PS_USE_CLOBBER_ARGV
 	/* If ps_buffer is a pointer, it might still be null */
 	if (!ps_buffer)
-		return;
+		return false;
 #endif
 
+	return true;
+}
+#endif							/* not PS_USE_NONE */
+
+/*
+ * set_ps_display_suffix
+ *		Adjust the process title to append 'suffix' onto the end with a space
+ *		between it and the current process title.
+ */
+void
+set_ps_display_suffix(const char *suffix)
+{
+#ifndef PS_USE_NONE
+	size_t		len;
+
+	/* first, check if we need to update the process title */
+	if (!update_ps_display_precheck())
+		return;
+
+	/* if there's already a suffix, overwrite it */
+	if (ps_buffer_nosuffix_len > 0)
+		ps_buffer_cur_len = ps_buffer_nosuffix_len;
+	else
+		ps_buffer_nosuffix_len = ps_buffer_cur_len;
+
+	len = strlen(suffix);
+
+	/* check if we have enough space to append the suffix */
+	if (ps_buffer_cur_len + len + 1 >= ps_buffer_size)
+	{
+		/* not enough space.  Check the buffer isn't full already */
+		if (ps_buffer_cur_len < ps_buffer_size - 1)
+		{
+			/* append a space before the suffix */
+			ps_buffer[ps_buffer_cur_len++] = ' ';
+
+			/* just add what we can and fill the ps_buffer */
+			memcpy(ps_buffer + ps_buffer_cur_len, suffix,
+				   ps_buffer_size - ps_buffer_cur_len - 1);
+			ps_buffer[ps_buffer_size - 1] = '\0';
+			ps_buffer_cur_len = ps_buffer_size - 1;
+		}
+	}
+	else
+	{
+		ps_buffer[ps_buffer_cur_len++] = ' ';
+		memcpy(ps_buffer + ps_buffer_cur_len, suffix, len + 1);
+		ps_buffer_cur_len = ps_buffer_cur_len + len;
+	}
+
+	Assert(strlen(ps_buffer) == ps_buffer_cur_len);
+
+	/* and set the new title */
+	flush_ps_display();
+#endif							/* not PS_USE_NONE */
+}
+
+/*
+ * set_ps_display_remove_suffix
+ *		Remove the process display suffix added by set_ps_display_suffix
+ */
+void
+set_ps_display_remove_suffix(void)
+{
+#ifndef PS_USE_NONE
+	/* first, check if we need to update the process title */
+	if (!update_ps_display_precheck())
+		return;
+
+	/* check we added a suffix */
+	if (ps_buffer_nosuffix_len == 0)
+		return;					/* no suffix */
+
+	/* remove the suffix from ps_buffer */
+	ps_buffer[ps_buffer_nosuffix_len] = '\0';
+	ps_buffer_cur_len = ps_buffer_nosuffix_len;
+	ps_buffer_nosuffix_len = 0;
+
+	Assert(ps_buffer_cur_len == strlen(ps_buffer));
+
+	/* and set the new title */
+	flush_ps_display();
+#endif							/* not PS_USE_NONE */
+}
+
+/*
+ * Call this to update the ps status display to a fixed prefix plus an
+ * indication of what you're currently doing passed in the argument.
+ *
+ * 'len' must be the same as strlen(activity)
+ */
+void
+set_ps_display_with_len(const char *activity, size_t len)
+{
+	Assert(strlen(activity) == len);
+
+#ifndef PS_USE_NONE
+	/* first, check if we need to update the process title */
+	if (!update_ps_display_precheck())
+		return;
+
+	/* wipe out any suffix when the title is completely changed */
+	ps_buffer_nosuffix_len = 0;
+
 	/* Update ps_buffer to contain both fixed part and activity */
-	strlcpy(ps_buffer + ps_buffer_fixed_size, activity,
-			ps_buffer_size - ps_buffer_fixed_size);
-	ps_buffer_cur_len = strlen(ps_buffer);
+	if (ps_buffer_fixed_size + len >= ps_buffer_size)
+	{
+		/* handle the case where ps_buffer doesn't have enough space */
+		memcpy(ps_buffer + ps_buffer_fixed_size, activity,
+			   ps_buffer_size - ps_buffer_fixed_size - 1);
+		ps_buffer[ps_buffer_size - 1] = '\0';
+		ps_buffer_cur_len = ps_buffer_size - 1;
+	}
+	else
+	{
+		memcpy(ps_buffer + ps_buffer_fixed_size, activity, len + 1);
+		ps_buffer_cur_len = ps_buffer_fixed_size + len;
+	}
+	Assert(strlen(ps_buffer) == ps_buffer_cur_len);
 
 	/* Transmit new setting to kernel, if necessary */
+	flush_ps_display();
+#endif							/* not PS_USE_NONE */
+}
 
+#ifndef PS_USE_NONE
+static void
+flush_ps_display(void)
+{
 #ifdef PS_USE_SETPROCTITLE
 	setproctitle("%s", ps_buffer);
 #elif defined(PS_USE_SETPROCTITLE_FAST)
@@ -363,9 +492,8 @@ set_ps_display(const char *activity)
 		ident_handle = CreateEvent(NULL, TRUE, FALSE, name);
 	}
 #endif							/* PS_USE_WIN32 */
-#endif							/* not PS_USE_NONE */
 }
-
+#endif							/* not PS_USE_NONE */
 
 /*
  * Returns what's currently in the ps display, in case someone needs

src/include/utils/ps_status.h

@@ -25,7 +25,22 @@ extern char **save_ps_display_args(int argc, char **argv);
 
 extern void init_ps_display(const char *fixed_part);
 
-extern void set_ps_display(const char *activity);
+extern void set_ps_display_suffix(const char *suffix);
+
+extern void set_ps_display_remove_suffix(void);
+
+extern void set_ps_display_with_len(const char *activity, size_t len);
+
+/*
+ * set_ps_display
+ *		inlined to allow strlen to be evaluated during compilation when
+ *		passing string constants.
+ */
+static inline void
+set_ps_display(const char *activity)
+{
+	set_ps_display_with_len(activity, strlen(activity));
+}
 
 extern const char *get_ps_display(int *displen);