public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v1] elog.c: Remove special case which avoided %*s format strings..
21+ messages / 2 participants
[nested] [flat]

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* [PATCH v1] elog.c: Remove special case which avoided %*s format strings..
@ 2020-08-01 02:53 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Justin Pryzby @ 2020-08-01 02:53 UTC (permalink / raw)

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

See also:
4334639f4 Allow printf-style padding specifications in log_line_prefix.
96bf88d52 Always use our own versions of *printf().
abd9ca377 Make assorted performance improvements in snprintf.c.
---
 src/backend/utils/error/elog.c | 134 ++++++++-------------------------
 1 file changed, 32 insertions(+), 102 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e..6b6749965f 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2350,11 +2350,6 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 		 * Note: Since only '-', '0' to '9' are valid formatting characters we
 		 * can do a quick check here to pre-check for formatting. If the char
 		 * is not formatting then we can skip a useless function call.
-		 *
-		 * Further note: At least on some platforms, passing %*s rather than
-		 * %s to appendStringInfo() is substantially slower, so many of the
-		 * cases below avoid doing that unless non-zero padding is in fact
-		 * specified.
 		 */
 		if (*p > '9')
 			padding = 0;
@@ -2371,10 +2366,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (appname == NULL || *appname == '\0')
 						appname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, appname);
-					else
-						appendStringInfoString(buf, appname);
+					appendStringInfo(buf, "%*s", padding, appname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2392,10 +2384,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					else
 						backend_type_str = GetBackendTypeDesc(MyBackendType);
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, backend_type_str);
-					else
-						appendStringInfoString(buf, backend_type_str);
+					appendStringInfo(buf, "%*s", padding, backend_type_str);
 					break;
 				}
 			case 'u':
@@ -2405,10 +2394,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (username == NULL || *username == '\0')
 						username = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, username);
-					else
-						appendStringInfoString(buf, username);
+					appendStringInfo(buf, "%*s", padding, username);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2421,17 +2407,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 
 					if (dbname == NULL || *dbname == '\0')
 						dbname = _("[unknown]");
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, dbname);
-					else
-						appendStringInfoString(buf, dbname);
+					appendStringInfo(buf, "%*s", padding, dbname);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'c':
-				if (padding != 0)
 				{
 					char		strfbuf[128];
 
@@ -2439,14 +2421,9 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) (MyStartTime), MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
-				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
 				break;
 			case 'p':
-				if (padding != 0)
-					appendStringInfo(buf, "%*d", padding, MyProcPid);
-				else
-					appendStringInfo(buf, "%d", MyProcPid);
+				appendStringInfo(buf, "%*d", padding, MyProcPid);
 				break;
 
 			case 'P':
@@ -2472,17 +2449,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 
 			case 'l':
-				if (padding != 0)
-					appendStringInfo(buf, "%*ld", padding, log_line_number);
-				else
-					appendStringInfo(buf, "%ld", log_line_number);
+				appendStringInfo(buf, "%*ld", padding, log_line_number);
 				break;
 			case 'm':
 				setup_formatted_log_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_log_time);
-				else
-					appendStringInfoString(buf, formatted_log_time);
+				appendStringInfo(buf, "%*s", padding, formatted_log_time);
 				break;
 			case 't':
 				{
@@ -2492,10 +2463,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					pg_strftime(strfbuf, sizeof(strfbuf),
 								"%Y-%m-%d %H:%M:%S %Z",
 								pg_localtime(&stamp_time, log_timezone));
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 'n':
@@ -2512,19 +2480,13 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 							 (long) saved_timeval.tv_sec,
 							 (int) (saved_timeval.tv_usec / 1000));
 
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, strfbuf);
-					else
-						appendStringInfoString(buf, strfbuf);
+					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				break;
 			case 's':
 				if (formatted_start_time[0] == '\0')
 					setup_formatted_start_time();
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, formatted_start_time);
-				else
-					appendStringInfoString(buf, formatted_start_time);
+				appendStringInfo(buf, "%*s", padding, formatted_start_time);
 				break;
 			case 'i':
 				if (MyProcPort)
@@ -2533,10 +2495,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 					int			displen;
 
 					psdisp = get_ps_display(&displen);
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, psdisp);
-					else
-						appendBinaryStringInfo(buf, psdisp, displen);
+					appendStringInfo(buf, "%*s", padding, psdisp);
 
 				}
 				else if (padding != 0)
@@ -2546,37 +2505,24 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 			case 'r':
 				if (MyProcPort && MyProcPort->remote_host)
 				{
-					if (padding != 0)
+					if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
 					{
-						if (MyProcPort->remote_port && MyProcPort->remote_port[0] != '\0')
-						{
-							/*
-							 * This option is slightly special as the port
-							 * number may be appended onto the end. Here we
-							 * need to build 1 string which contains the
-							 * remote_host and optionally the remote_port (if
-							 * set) so we can properly align the string.
-							 */
-
-							char	   *hostport;
-
-							hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
-							appendStringInfo(buf, "%*s", padding, hostport);
-							pfree(hostport);
-						}
-						else
-							appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
+						/*
+						 * This option is slightly special as the port
+						 * number may be appended onto the end. Here we
+						 * need to build 1 string which contains the
+						 * remote_host and optionally the remote_port (if
+						 * set) so we can properly align the string.
+						 */
+
+						char	   *hostport;
+
+						hostport = psprintf("%s(%s)", MyProcPort->remote_host, MyProcPort->remote_port);
+						appendStringInfo(buf, "%*s", padding, hostport);
+						pfree(hostport);
 					}
 					else
-					{
-						/* padding is 0, so we don't need a temp buffer */
-						appendStringInfoString(buf, MyProcPort->remote_host);
-						if (MyProcPort->remote_port &&
-							MyProcPort->remote_port[0] != '\0')
-							appendStringInfo(buf, "(%s)",
-											 MyProcPort->remote_port);
-					}
-
+						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
@@ -2584,12 +2530,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				break;
 			case 'h':
 				if (MyProcPort && MyProcPort->remote_host)
-				{
-					if (padding != 0)
-						appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
-					else
-						appendStringInfoString(buf, MyProcPort->remote_host);
-				}
+					appendStringInfo(buf, "%*s", padding, MyProcPort->remote_host);
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
@@ -2604,32 +2545,21 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
 				/* keep VXID format in sync with lockfuncs.c */
 				if (MyProc != NULL && MyProc->backendId != InvalidBackendId)
 				{
-					if (padding != 0)
-					{
-						char		strfbuf[128];
+					char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
-								 MyProc->backendId, MyProc->lxid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+							 MyProc->backendId, MyProc->lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
-					}
-					else
-						appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
 										   padding > 0 ? padding : -padding);
 				break;
 			case 'x':
-				if (padding != 0)
-					appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
-				else
-					appendStringInfo(buf, "%u", GetTopTransactionIdIfAny());
+				appendStringInfo(buf, "%*u", padding, GetTopTransactionIdIfAny());
 				break;
 			case 'e':
-				if (padding != 0)
-					appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
-				else
-					appendStringInfoString(buf, unpack_sql_state(edata->sqlerrcode));
+				appendStringInfo(buf, "%*s", padding, unpack_sql_state(edata->sqlerrcode));
 				break;
 			default:
 				/* format error - ignore it */
-- 
2.17.0


--pAwQNkOnpTn9IO2O--





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

* Better documentation for RIR term?
@ 2024-08-22 04:51 Kirill Reshke <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Kirill Reshke @ 2024-08-22 04:51 UTC (permalink / raw)
  To: pgsql-hackers

I have had hard times understanding what RIR rules are while reading
some threads in pgsql-hackers. One example is 'Virtual generated
columns'. I was trying to grep 'RIR rules', when I just started to
read kernel sources. And I only today I finally bumped into this note
in rewriteHandler.c

```
* NOTES
* Some of the terms used in this file are of historic nature: "retrieve"
* was the PostQUEL keyword for what today is SELECT. "RIR" stands for
* "Retrieve-Instead-Retrieve", that is an ON SELECT DO INSTEAD SELECT rule
* (which has to be unconditional and where only one rule can exist on each
* relation).
*

```

Maybe I'm really bad at searching things and nobody before had
problems understanding what RIR stands for. If not, should we enhance
documentation in some way? If yes, what is the proper place?

-- 
Best regards,
Kirill Reshke






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

end of thread, other threads:[~2024-08-22 04:51 UTC | newest]

Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2020-08-01 02:53 [PATCH v1] elog.c: Remove special case which avoided %*s format strings.. Justin Pryzby <[email protected]>
2024-08-22 04:51 Better documentation for RIR term? Kirill Reshke <[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