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
Date: Tue, 16 Oct 2018 01:59:00 -0400
Message-ID: <6813.1539669540@sss.pgh.pa.us> (raw)
In-Reply-To: <31562.1539658872@sss.pgh.pa.us>
References: <20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de>
<31562.1539658872@sss.pgh.pa.us>
I wrote:
> 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.
Hm, looking at that more closely, there's a problem with the idea of
allocating the cache slots one-at-a-time. Currently,
sizeof(DCHCacheEntry) and sizeof(NUMCacheEntry) are each just a bit more
than a power of 2, which would cause palloc to waste nearly 50% of the
allocation it makes for them.
We could forget the one-at-a-time idea and just allocate the whole
array on first use, but I feel like that's probably not a good answer.
I'm inclined to instead reduce DCH_CACHE_SIZE and NUM_CACHE_SIZE enough
to make the structs just less than powers of 2 instead of just more ---
AFAICS, both of those numbers were picked out of the air, and so there's
no reason not to pick a different number out of the air.
Also, I noticed that the biggest part of those structs are arrays of
FormatNode, which has been designed with complete lack of thought about
size or padding issues. We can very easily cut it in half on 64-bit
machines.
The attached patch does both of those things. It's independent of
the other one but is important to make that one efficient.
regards, tom lane
Attachments:formatting-shave-bits.patch (text/x-diff, 2.8K) downloaddiff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.cindex b3ff133..9c2649e 100644
*** a/src/backend/utils/adt/formatting.c
--- b/src/backend/utils/adt/formatting.c
***************
*** 124,133 ****
*/
typedef struct
{
! char *name; /* suffix string */
int len, /* suffix length */
id, /* used in node->suffix */
! type; /* prefix / postfix */
} KeySuffix;
/* ----------
--- 124,133 ----
*/
typedef struct
{
! const char *name; /* suffix string */
int len, /* suffix length */
id, /* used in node->suffix */
! type; /* prefix / postfix */
} KeySuffix;
/* ----------
*************** typedef struct
*** 155,164 ****
typedef struct
{
! int type; /* NODE_TYPE_XXX, see below */
! const KeyWord *key; /* if type is ACTION */
char character[MAX_MULTIBYTE_CHAR_LEN + 1]; /* if type is CHAR */
! int suffix; /* keyword prefix/suffix code, if any */
} FormatNode;
#define NODE_TYPE_END 1
--- 155,164 ----
typedef struct
{
! uint8 type; /* NODE_TYPE_XXX, see below */
char character[MAX_MULTIBYTE_CHAR_LEN + 1]; /* if type is CHAR */
! uint8 suffix; /* keyword prefix/suffix code, if any */
! const KeyWord *key; /* if type is ACTION */
} FormatNode;
#define NODE_TYPE_END 1
*************** typedef struct
*** 358,370 ****
* For simplicity, the cache entries are fixed-size, so they allow for the
* worst case of a FormatNode for each byte in the picture string.
*
* The max number of entries in the caches is DCH_CACHE_ENTRIES
* resp. NUM_CACHE_ENTRIES.
* ----------
*/
! #define NUM_CACHE_SIZE 64
#define NUM_CACHE_ENTRIES 20
! #define DCH_CACHE_SIZE 128
#define DCH_CACHE_ENTRIES 20
typedef struct
--- 358,374 ----
* For simplicity, the cache entries are fixed-size, so they allow for the
* worst case of a FormatNode for each byte in the picture string.
*
+ * The CACHE_SIZE constants are chosen to make sizeof(DCHCacheEntry) and+ * sizeof(NUMCacheEntry) be powers of 2, or just less than that, so that+ * we don't waste too much space by palloc'ing them individually.+ *
* The max number of entries in the caches is DCH_CACHE_ENTRIES
* resp. NUM_CACHE_ENTRIES.
* ----------
*/
! #define NUM_CACHE_SIZE 56
#define NUM_CACHE_ENTRIES 20
! #define DCH_CACHE_SIZE 119
#define DCH_CACHE_ENTRIES 20
typedef struct
*************** do { \
*** 496,502 ****
*****************************************************************************/
/* ----------
! * Suffixes:
* ----------
*/
#define DCH_S_FM 0x01
--- 500,506 ----
*****************************************************************************/
/* ----------
! * Suffixes (FormatNode.suffix is an OR of these codes)
* ----------
*/
#define DCH_S_FM 0x01
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, thomas.munro@enterprisedb.com
Subject: Re: Large writable variables
In-Reply-To: <6813.1539669540@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox