From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@enterprisedb.com>
Subject: Re: Large writable variables
In-reply-to: <20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de>
References: <20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Mon, 15 Oct 2018 13:07:54 -0700"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <31510.1539658813.0@sss.pgh.pa.us>
Date: Mon, 15 Oct 2018 23:01:12 -0400
Message-ID: <31562.1539658872@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <31510.1539658813.1@sss.pgh.pa.us>

Andres Freund <andres@anarazel.de> writes:
> top unitialized allocations:
> 0000000008435040 0000000000085280 b DCHCache
> 0000000008391168 0000000000043840 b NUMCache

Here's a patch to improve that situation.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="alloc-formatting-cache-only-as-needed.patch";
	charset="us-ascii"
Content-ID: <31510.1539658813.2@sss.pgh.pa.us>
Content-Description: alloc-formatting-cache-only-as-needed.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/fo=
rmatting.c
index b3ff133..43a0307 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -93,6 +93,7 @@
 #include "utils/float.h"
 #include "utils/formatting.h"
 #include "utils/int8.h"
+#include "utils/memutils.h"
 #include "utils/numeric.h"
 #include "utils/pg_locale.h"
 =

@@ -385,12 +386,12 @@ typedef struct
 } NUMCacheEntry;
 =

 /* global cache for date/time format pictures */
-static DCHCacheEntry DCHCache[DCH_CACHE_ENTRIES];
+static DCHCacheEntry *DCHCache[DCH_CACHE_ENTRIES];
 static int	n_DCHCache =3D 0;		/* current number of entries */
 static int	DCHCounter =3D 0;		/* aging-event counter */
 =

 /* global cache for number format pictures */
-static NUMCacheEntry NUMCache[NUM_CACHE_ENTRIES];
+static NUMCacheEntry *NUMCache[NUM_CACHE_ENTRIES];
 static int	n_NUMCache =3D 0;		/* current number of entries */
 static int	NUMCounter =3D 0;		/* aging-event counter */
 =

@@ -3368,13 +3369,13 @@ DCH_cache_getnew(const char *str)
 {
 	DCHCacheEntry *ent;
 =

-	/* counter overflow check - paranoia? */
+	/* handle counter overflow by resetting all ages */
 	if (DCHCounter >=3D (INT_MAX - DCH_CACHE_ENTRIES))
 	{
 		DCHCounter =3D 0;
 =

-		for (ent =3D DCHCache; ent < (DCHCache + DCH_CACHE_ENTRIES); ent++)
-			ent->age =3D (++DCHCounter);
+		for (int i =3D 0; i < n_DCHCache; i++)
+			DCHCache[i]->age =3D (++DCHCounter);
 	}
 =

 	/*
@@ -3382,15 +3383,16 @@ DCH_cache_getnew(const char *str)
 	 */
 	if (n_DCHCache >=3D DCH_CACHE_ENTRIES)
 	{
-		DCHCacheEntry *old =3D DCHCache + 0;
+		DCHCacheEntry *old =3D DCHCache[0];
 =

 #ifdef DEBUG_TO_FROM_CHAR
 		elog(DEBUG_elog_output, "cache is full (%d)", n_DCHCache);
 #endif
 		if (old->valid)
 		{
-			for (ent =3D DCHCache + 1; ent < (DCHCache + DCH_CACHE_ENTRIES); ent++=
)
+			for (int i =3D 1; i < DCH_CACHE_ENTRIES; i++)
 			{
+				ent =3D DCHCache[i];
 				if (!ent->valid)
 				{
 					old =3D ent;
@@ -3414,7 +3416,9 @@ DCH_cache_getnew(const char *str)
 #ifdef DEBUG_TO_FROM_CHAR
 		elog(DEBUG_elog_output, "NEW (%d)", n_DCHCache);
 #endif
-		ent =3D DCHCache + n_DCHCache;
+		Assert(DCHCache[n_DCHCache] =3D=3D NULL);
+		DCHCache[n_DCHCache] =3D ent =3D (DCHCacheEntry *)
+			MemoryContextAllocZero(TopMemoryContext, sizeof(DCHCacheEntry));
 		ent->valid =3D false;
 		StrNCpy(ent->str, str, DCH_CACHE_SIZE + 1);
 		ent->age =3D (++DCHCounter);
@@ -3428,20 +3432,19 @@ DCH_cache_getnew(const char *str)
 static DCHCacheEntry *
 DCH_cache_search(const char *str)
 {
-	int			i;
-	DCHCacheEntry *ent;
-
-	/* counter overflow check - paranoia? */
+	/* handle counter overflow by resetting all ages */
 	if (DCHCounter >=3D (INT_MAX - DCH_CACHE_ENTRIES))
 	{
 		DCHCounter =3D 0;
 =

-		for (ent =3D DCHCache; ent < (DCHCache + DCH_CACHE_ENTRIES); ent++)
-			ent->age =3D (++DCHCounter);
+		for (int i =3D 0; i < n_DCHCache; i++)
+			DCHCache[i]->age =3D (++DCHCounter);
 	}
 =

-	for (i =3D 0, ent =3D DCHCache; i < n_DCHCache; i++, ent++)
+	for (int i =3D 0; i < n_DCHCache; i++)
 	{
+		DCHCacheEntry *ent =3D DCHCache[i];
+
 		if (ent->valid && strcmp(ent->str, str) =3D=3D 0)
 		{
 			ent->age =3D (++DCHCounter);
@@ -4047,13 +4050,13 @@ NUM_cache_getnew(const char *str)
 {
 	NUMCacheEntry *ent;
 =

-	/* counter overflow check - paranoia? */
+	/* handle counter overflow by resetting all ages */
 	if (NUMCounter >=3D (INT_MAX - NUM_CACHE_ENTRIES))
 	{
 		NUMCounter =3D 0;
 =

-		for (ent =3D NUMCache; ent < (NUMCache + NUM_CACHE_ENTRIES); ent++)
-			ent->age =3D (++NUMCounter);
+		for (int i =3D 0; i < n_NUMCache; i++)
+			NUMCache[i]->age =3D (++NUMCounter);
 	}
 =

 	/*
@@ -4061,15 +4064,16 @@ NUM_cache_getnew(const char *str)
 	 */
 	if (n_NUMCache >=3D NUM_CACHE_ENTRIES)
 	{
-		NUMCacheEntry *old =3D NUMCache + 0;
+		NUMCacheEntry *old =3D NUMCache[0];
 =

 #ifdef DEBUG_TO_FROM_CHAR
 		elog(DEBUG_elog_output, "Cache is full (%d)", n_NUMCache);
 #endif
 		if (old->valid)
 		{
-			for (ent =3D NUMCache + 1; ent < (NUMCache + NUM_CACHE_ENTRIES); ent++=
)
+			for (int i =3D 1; i < NUM_CACHE_ENTRIES; i++)
 			{
+				ent =3D NUMCache[i];
 				if (!ent->valid)
 				{
 					old =3D ent;
@@ -4093,7 +4097,9 @@ NUM_cache_getnew(const char *str)
 #ifdef DEBUG_TO_FROM_CHAR
 		elog(DEBUG_elog_output, "NEW (%d)", n_NUMCache);
 #endif
-		ent =3D NUMCache + n_NUMCache;
+		Assert(NUMCache[n_NUMCache] =3D=3D NULL);
+		NUMCache[n_NUMCache] =3D ent =3D (NUMCacheEntry *)
+			MemoryContextAllocZero(TopMemoryContext, sizeof(NUMCacheEntry));
 		ent->valid =3D false;
 		StrNCpy(ent->str, str, NUM_CACHE_SIZE + 1);
 		ent->age =3D (++NUMCounter);
@@ -4107,20 +4113,19 @@ NUM_cache_getnew(const char *str)
 static NUMCacheEntry *
 NUM_cache_search(const char *str)
 {
-	int			i;
-	NUMCacheEntry *ent;
-
-	/* counter overflow check - paranoia? */
+	/* handle counter overflow by resetting all ages */
 	if (NUMCounter >=3D (INT_MAX - NUM_CACHE_ENTRIES))
 	{
 		NUMCounter =3D 0;
 =

-		for (ent =3D NUMCache; ent < (NUMCache + NUM_CACHE_ENTRIES); ent++)
-			ent->age =3D (++NUMCounter);
+		for (int i =3D 0; i < n_NUMCache; i++)
+			NUMCache[i]->age =3D (++NUMCounter);
 	}
 =

-	for (i =3D 0, ent =3D NUMCache; i < n_NUMCache; i++, ent++)
+	for (int i =3D 0; i < n_NUMCache; i++)
 	{
+		NUMCacheEntry *ent =3D NUMCache[i];
+
 		if (ent->valid && strcmp(ent->str, str) =3D=3D 0)
 		{
 			ent->age =3D (++NUMCounter);

------- =_aaaaaaaaaa0--