Re: Large writable variables - Mailing list pgsql-hackers

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

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 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