Re: Lisp_Marker size on 32bit systems

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

Stefan Monnier wrote:

> Thanks.  AFAICT the only solution is to use the GCALIGNED_UNION trick in
> each and every "real Lisp_Object struct" rather than once and forall in
> vectorlike_header.

The trick does need to move out of union vectorlike_header. However, the trick 
is not needed for most of those structs, since they're allocated only by the GC 
and are therefore already GC-aligned. The trick is needed only for structs that 
C might allocate statically or on the stack, and whose addresses are tagged as 
Lisp pointers. Just a few types do that, and I've noted them in the first 
attached patch.

Although the first attached patch shrinks sizeof (struct Lisp_Maker) from 32 to 
24 bytes on x86 as requested, allocate_pseudovector still *allocates* 32 bytes 
for the struct, as it rounds the size up to the next multiple of alignof 
(max_align_t), which is 16 on x86. It's not hard to change that to 8 (please see 
2nd attached patch) but this causes a 20% CPU performance hit (!) to 'make 
compile-always' on my platform (AMD Phenom II X4 910e circa 2010, Fedora 28 
x86-64, gcc -m32 -march=native), so I didn't install and can't recommend the 2nd 
attached patch.

[-- Attachment #2: 0001-Shrink-pseudovectors-a-bit.patch --]
[-- Type: text/x-patch, Size: 15465 bytes --]

From 2ccf72b1af5eef8746b9b6facb7c09e6258afb90 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 6 Sep 2018 19:17:14 -0700
Subject: [PATCH] Shrink pseudovectors a bit

sizeof (struct Lisp_Marker) was 32 on x86, where 24 would do.
Problem noted by Stefan Monnier in:
https://lists.gnu.org/r/emacs-devel/2018-09/msg00165.html
* src/bignum.h (struct Lisp_Bignum):
* src/frame.h (struct frame):
* src/lisp.h (struct Lisp_Vector, struct Lisp_Bool_Vector)
(struct Lisp_Char_Table, struct Lisp_Hash_Table)
(struct Lisp_Marker, struct Lisp_Overlay)
(struct Lisp_Misc_Ptr, struct Lisp_User_Ptr)
(struct Lisp_Finalizer, struct Lisp_Float)
(struct Lisp_Module_Function):
* src/process.h (struct Lisp_Process):
* src/termhooks.h (struct terminal):
* src/thread.h (struct thread_state, struct Lisp_Mutex)
(struct Lisp_CondVar):
* src/window.c (struct save_window_data):
* src/window.h (struct window):
* src/xterm.h (struct scroll_bar):
* src/xwidget.h (struct xwidget, struct xwidget_view):
Add GCALIGNED_STRUCT attribute.
* src/lisp.h (GCALIGNED_UNION_MEMBER): Renamed from GCALIGNED_UNION.
All uses changed.
(GCALIGNED_STRUCT_MEMBER, GCALIGNED_STRUCT, GCALIGNED): New macros.
All uses of open-coded GCALIGNED changed to use GCALIGNED.
(union vectorlike_header): No longer GC-aligned.
(PSEUDOVECSIZE): Yield 0 for pseudovectors without Lisp
objects that place a member before where the first Lisp object
member would be.
---
 src/alloc.c     |  8 +++--
 src/bignum.h    |  2 +-
 src/fileio.c    |  4 +--
 src/frame.h     |  2 +-
 src/keymap.c    |  4 +--
 src/lisp.h      | 90 ++++++++++++++++++++++++++++++-------------------
 src/process.h   |  2 +-
 src/termhooks.h |  2 +-
 src/thread.h    |  6 ++--
 src/window.c    |  2 +-
 src/window.h    |  2 +-
 src/xterm.h     |  2 +-
 src/xwidget.h   |  4 +--
 13 files changed, 76 insertions(+), 54 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 28ca7804ee..abb98a9eb6 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -641,9 +641,11 @@ buffer_memory_full (ptrdiff_t nbytes)
    implement Lisp objects; since pseudovectors can contain any C type,
    this is max_align_t.  On recent GNU/Linux x86 and x86-64 this can
    often waste up to 8 bytes, since alignof (max_align_t) is 16 but
-   typical vectors need only an alignment of 8.  However, it is not
-   worth the hassle to avoid this waste.  */
-enum { LISP_ALIGNMENT = alignof (union { max_align_t x; GCALIGNED_UNION }) };
+   typical vectors need only an alignment of 8.  Although shrinking
+   the alignment to 8 would save memory, it cost a 20% hit to Emacs
+   CPU performance on Fedora 28 x86-64 when compiled with gcc -m32.  */
+enum { LISP_ALIGNMENT = alignof (union { max_align_t x;
+					 GCALIGNED_UNION_MEMBER }) };
 verify (LISP_ALIGNMENT % GCALIGNMENT == 0);
 
 /* True if malloc (N) is known to return storage suitably aligned for
diff --git a/src/bignum.h b/src/bignum.h
index 0e38c615ee..6551549343 100644
--- a/src/bignum.h
+++ b/src/bignum.h
@@ -39,7 +39,7 @@ struct Lisp_Bignum
 {
   union vectorlike_header header;
   mpz_t value;
-};
+} GCALIGNED_STRUCT;
 
 extern mpz_t mpz[4];
 
diff --git a/src/fileio.c b/src/fileio.c
index 66b2333317..5ca7c595f7 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3394,9 +3394,9 @@ union read_non_regular
     int fd;
     ptrdiff_t inserted, trytry;
   } s;
-  GCALIGNED_UNION
+  GCALIGNED_UNION_MEMBER
 };
-verify (alignof (union read_non_regular) % GCALIGNMENT == 0);
+verify (GCALIGNED (union read_non_regular));
 
 static Lisp_Object
 read_non_regular (Lisp_Object state)
diff --git a/src/frame.h b/src/frame.h
index a3bb633e57..ad7376a653 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -578,7 +578,7 @@ struct frame
   enum ns_appearance_type ns_appearance;
   bool_bf ns_transparent_titlebar;
 #endif
-};
+} GCALIGNED_STRUCT;
 
 /* Most code should use these functions to set Lisp fields in struct frame.  */
 
diff --git a/src/keymap.c b/src/keymap.c
index 52db7b491f..79dce15a81 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -554,9 +554,9 @@ union map_keymap
     Lisp_Object args;
     void *data;
   } s;
-  GCALIGNED_UNION
+  GCALIGNED_UNION_MEMBER
 };
-verify (alignof (union map_keymap) % GCALIGNMENT == 0);
+verify (GCALIGNED (union map_keymap));
 
 static void
 map_keymap_char_table_item (Lisp_Object args, Lisp_Object key, Lisp_Object val)
diff --git a/src/lisp.h b/src/lisp.h
index 78c25f97dc..7e365e8f47 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -229,7 +229,7 @@ extern bool suppress_checking EXTERNALLY_VISIBLE;
    USE_LSB_TAG not only requires the least 3 bits of pointers returned by
    malloc to be 0 but also needs to be able to impose a mult-of-8 alignment
    on some non-GC Lisp_Objects, all of which are aligned via
-   GCALIGNED_UNION at the end of a union.  */
+   GCALIGNED_UNION_MEMBER, GCALIGNED_STRUCT_MEMBER, and GCALIGNED_STRUCT.  */
 
 enum Lisp_Bits
   {
@@ -282,7 +282,35 @@ error !;
 # define GCALIGNMENT 1
 #endif
 
-#define GCALIGNED_UNION char alignas (GCALIGNMENT) gcaligned;
+/* If a struct is always allocated by the GC and is therefore always
+   GC-aligned, put GCALIGNED_STRUCT after its closing '}'; this can
+   help the compiler generate better code.
+
+   To cause a union to have alignment of at least GCALIGNMENT, put
+   GCALIGNED_UNION_MEMBER in its member list.  Similarly for a struct
+   and GCALIGNED_STRUCT_MEMBER, although this may make the struct a
+   bit bigger on non-GCC platforms.  Any struct using
+   GCALIGNED_STRUCT_MEMBER should also use GCALIGNED_STRUCT.
+
+   Although these macros are reasonably portable, they are not
+   guaranteed on non-GCC platforms, as C11 does not require support
+   for alignment to GCALIGNMENT and older compilers may ignore
+   alignment requests.  For any type T where garbage collection
+   requires alignment, use verify (GCALIGNED (T)) to verify the
+   requirement on the current platform.  Types need this check if
+   their objects can be allocated outside the garbage collector.  For
+   example, struct Lisp_Symbol needs the check because of lispsym and
+   struct Lisp_Cons needs it because of STACK_CONS.  */
+
+#define GCALIGNED_UNION_MEMBER char alignas (GCALIGNMENT) gcaligned;
+#if HAVE_STRUCT_ATTRIBUTE_ALIGNED
+# define GCALIGNED_STRUCT_MEMBER
+# define GCALIGNED_STRUCT __attribute__ ((aligned (GCALIGNMENT)))
+#else
+# define GCALIGNED_STRUCT_MEMBER GCALIGNED_UNION_MEMBER
+# define GCALIGNED_STRUCT
+#endif
+#define GCALIGNED(type) (alignof (type) % GCALIGNMENT == 0)
 
 /* Lisp_Word is a scalar word suitable for holding a tagged pointer or
    integer.  Usually it is a pointer to a deliberately-incomplete type
@@ -751,10 +779,10 @@ struct Lisp_Symbol
       /* Next symbol in obarray bucket, if the symbol is interned.  */
       struct Lisp_Symbol *next;
     } s;
-    GCALIGNED_UNION
+    GCALIGNED_UNION_MEMBER
   } u;
 };
-verify (alignof (struct Lisp_Symbol) % GCALIGNMENT == 0);
+verify (GCALIGNED (struct Lisp_Symbol));
 
 /* Declare a Lisp-callable function.  The MAXARGS parameter has the same
    meaning as in the DEFUN macro, and is used to construct a prototype.  */
@@ -843,7 +871,9 @@ typedef EMACS_UINT Lisp_Word_tag;
    and PSEUDOVECTORP cast their pointers to union vectorlike_header *,
    because when two such pointers potentially alias, a compiler won't
    incorrectly reorder loads and stores to their size fields.  See
-   Bug#8546.  */
+   Bug#8546.  This union formerly contained more members, and there's
+   no compelling reason to change it to a struct merely because the
+   number of members has been reduced to one.  */
 union vectorlike_header
   {
     /* The main member contains various pieces of information:
@@ -866,20 +896,7 @@ union vectorlike_header
 	 Current layout limits the pseudovectors to 63 PVEC_xxx subtypes,
 	 4095 Lisp_Objects in GC-ed area and 4095 word-sized other slots.  */
     ptrdiff_t size;
-    /* Align the union so that there is no padding after it.
-       This is needed for the following reason:
-       If the alignment constraint of Lisp_Object is greater than the size of
-       vectorlike_header (e.g. with-wide-int), vectorlike objects which have
-       0 Lisp_Object fields and whose 1st field has a smaller alignment
-       constraint than Lisp_Object may end up with their 1st field "before
-       pseudovector index 0", in which case PSEUDOVECSIZE will return
-       a "negative" number.  We could fix PSEUDOVECSIZE, but it's easier to
-       just force rounding up the size of vectorlike_header to the alignment
-       of Lisp_Object.  */
-    Lisp_Object align;
-    GCALIGNED_UNION
   };
-verify (alignof (union vectorlike_header) % GCALIGNMENT == 0);
 
 INLINE bool
 (SYMBOLP) (Lisp_Object x)
@@ -1251,10 +1268,10 @@ struct Lisp_Cons
 	struct Lisp_Cons *chain;
       } u;
     } s;
-    GCALIGNED_UNION
+    GCALIGNED_UNION_MEMBER
   } u;
 };
-verify (alignof (struct Lisp_Cons) % GCALIGNMENT == 0);
+verify (GCALIGNED (struct Lisp_Cons));
 
 INLINE bool
 (NILP) (Lisp_Object x)
@@ -1373,10 +1390,10 @@ struct Lisp_String
       unsigned char *data;
     } s;
     struct Lisp_String *next;
-    GCALIGNED_UNION
+    GCALIGNED_UNION_MEMBER
   } u;
 };
-verify (alignof (struct Lisp_String) % GCALIGNMENT == 0);
+verify (GCALIGNED (struct Lisp_String));
 
 INLINE bool
 STRINGP (Lisp_Object x)
@@ -1507,7 +1524,7 @@ struct Lisp_Vector
   {
     union vectorlike_header header;
     Lisp_Object contents[FLEXIBLE_ARRAY_MEMBER];
-  };
+  } GCALIGNED_STRUCT;
 
 INLINE bool
 (VECTORLIKEP) (Lisp_Object x)
@@ -1599,7 +1616,7 @@ struct Lisp_Bool_Vector
        The bits are in little-endian order in the bytes, and
        the bytes are in little-endian order in the words.  */
     bits_word data[FLEXIBLE_ARRAY_MEMBER];
-  };
+  } GCALIGNED_STRUCT;
 
 /* Some handy constants for calculating sizes
    and offsets, mostly of vectorlike objects.   */
@@ -1765,7 +1782,8 @@ memclear (void *p, ptrdiff_t nbytes)
    ones that the GC needs to trace).  */
 
 #define PSEUDOVECSIZE(type, nonlispfield)			\
-  ((offsetof (type, nonlispfield) - header_size) / word_size)
+   (offsetof (type, nonlispfield) < header_size			\
+    ? 0 : (offsetof (type, nonlispfield) - header_size) / word_size)
 
 /* Compute A OP B, using the unsigned comparison operator OP.  A and B
    should be integer expressions.  This is not the same as
@@ -1830,7 +1848,7 @@ struct Lisp_Char_Table
 
     /* These hold additional data.  It is a vector.  */
     Lisp_Object extras[FLEXIBLE_ARRAY_MEMBER];
-  };
+  } GCALIGNED_STRUCT;
 
 INLINE bool
 CHAR_TABLE_P (Lisp_Object a)
@@ -1942,7 +1960,9 @@ struct Lisp_Subr
     const char *symbol_name;
     const char *intspec;
     EMACS_INT doc;
-  };
+    GCALIGNED_STRUCT_MEMBER
+  } GCALIGNED_STRUCT;
+verify (GCALIGNED (struct Lisp_Subr));
 
 INLINE bool
 SUBRP (Lisp_Object a)
@@ -2194,7 +2214,7 @@ struct Lisp_Hash_Table
   /* Next weak hash table if this is a weak hash table.  The head
      of the list is in weak_hash_tables.  */
   struct Lisp_Hash_Table *next_weak;
-};
+} GCALIGNED_STRUCT;
 
 
 INLINE bool
@@ -2313,7 +2333,7 @@ struct Lisp_Marker
      used to implement the functionality of markers, but rather to (ab)use
      markers as a cache for char<->byte mappings).  */
   ptrdiff_t bytepos;
-};
+} GCALIGNED_STRUCT;
 
 /* START and END are markers in the overlay's buffer, and
    PLIST is the overlay's property list.  */
@@ -2335,13 +2355,13 @@ struct Lisp_Overlay
     Lisp_Object end;
     Lisp_Object plist;
     struct Lisp_Overlay *next;
-  };
+  } GCALIGNED_STRUCT;
 
 struct Lisp_Misc_Ptr
   {
     union vectorlike_header header;
     void *pointer;
-  };
+  } GCALIGNED_STRUCT;
 
 extern Lisp_Object make_misc_ptr (void *);
 
@@ -2388,7 +2408,7 @@ struct Lisp_User_Ptr
   union vectorlike_header header;
   void (*finalizer) (void *);
   void *p;
-};
+} GCALIGNED_STRUCT;
 #endif
 
 /* A finalizer sentinel.  */
@@ -2404,7 +2424,7 @@ struct Lisp_Finalizer
     /* Circular list of all active weak references.  */
     struct Lisp_Finalizer *prev;
     struct Lisp_Finalizer *next;
-  };
+  } GCALIGNED_STRUCT;
 
 INLINE bool
 FINALIZERP (Lisp_Object x)
@@ -2616,7 +2636,7 @@ struct Lisp_Float
       double data;
       struct Lisp_Float *chain;
     } u;
-  };
+  } GCALIGNED_STRUCT;
 
 INLINE bool
 (FLOATP) (Lisp_Object x)
@@ -3946,7 +3966,7 @@ struct Lisp_Module_Function
   ptrdiff_t min_arity, max_arity;
   emacs_subr subr;
   void *data;
-};
+} GCALIGNED_STRUCT;
 
 INLINE bool
 MODULE_FUNCTIONP (Lisp_Object o)
diff --git a/src/process.h b/src/process.h
index 6bc22146a7..3c6dd7b91f 100644
--- a/src/process.h
+++ b/src/process.h
@@ -203,7 +203,7 @@ struct Lisp_Process
     bool_bf gnutls_p : 1;
     bool_bf gnutls_complete_negotiation_p : 1;
 #endif
-};
+  } GCALIGNED_STRUCT;
 
 INLINE bool
 PROCESSP (Lisp_Object a)
diff --git a/src/termhooks.h b/src/termhooks.h
index 8b5f648b43..211429169b 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -661,7 +661,7 @@ struct terminal
      frames on the terminal when it calls this hook, so infinite
      recursion is prevented.  */
   void (*delete_terminal_hook) (struct terminal *);
-};
+} GCALIGNED_STRUCT;
 
 INLINE bool
 TERMINALP (Lisp_Object a)
diff --git a/src/thread.h b/src/thread.h
index 8ecb00824d..28d8d864fb 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -184,7 +184,7 @@ struct thread_state
 
   /* Threads are kept on a linked list.  */
   struct thread_state *next_thread;
-};
+} GCALIGNED_STRUCT;
 
 INLINE bool
 THREADP (Lisp_Object a)
@@ -231,7 +231,7 @@ struct Lisp_Mutex
 
   /* The lower-level mutex object.  */
   lisp_mutex_t mutex;
-};
+} GCALIGNED_STRUCT;
 
 INLINE bool
 MUTEXP (Lisp_Object a)
@@ -265,7 +265,7 @@ struct Lisp_CondVar
 
   /* The lower-level condition variable object.  */
   sys_cond_t cond;
-};
+} GCALIGNED_STRUCT;
 
 INLINE bool
 CONDVARP (Lisp_Object a)
diff --git a/src/window.c b/src/window.c
index d4fc5568a5..04de965680 100644
--- a/src/window.c
+++ b/src/window.c
@@ -6268,7 +6268,7 @@ struct save_window_data
     /* These are currently unused.  We need them as soon as we convert
        to pixels.  */
     int frame_menu_bar_height, frame_tool_bar_height;
-  };
+  } GCALIGNED_STRUCT;
 
 /* This is saved as a Lisp_Vector.  */
 struct saved_window
diff --git a/src/window.h b/src/window.h
index 013083eb9a..cc0b6b6667 100644
--- a/src/window.h
+++ b/src/window.h
@@ -400,7 +400,7 @@ struct window
     /* Z_BYTE - buffer position of the last glyph in the current matrix of W.
        Should be nonnegative, and only valid if window_end_valid is true.  */
     ptrdiff_t window_end_bytepos;
-  };
+  } GCALIGNED_STRUCT;
 
 INLINE bool
 WINDOWP (Lisp_Object a)
diff --git a/src/xterm.h b/src/xterm.h
index 1849a5c953..2ea8a93f8c 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -937,7 +937,7 @@ struct scroll_bar
 
   /* True if the scroll bar is horizontal.  */
   bool horizontal;
-};
+} GCALIGNED_STRUCT;
 
 /* Turning a lisp vector value into a pointer to a struct scroll_bar.  */
 #define XSCROLL_BAR(vec) ((struct scroll_bar *) XVECTOR (vec))
diff --git a/src/xwidget.h b/src/xwidget.h
index 89fc7ff458..c203d4f60c 100644
--- a/src/xwidget.h
+++ b/src/xwidget.h
@@ -61,7 +61,7 @@ struct xwidget
 
   /* Kill silently if Emacs is exited.  */
   bool_bf kill_without_query : 1;
-};
+} GCALIGNED_STRUCT;
 
 struct xwidget_view
 {
@@ -88,7 +88,7 @@ struct xwidget_view
   int clip_left;
 
   long handler_id;
-};
+} GCALIGNED_STRUCT;
 #endif
 
 /* Test for xwidget pseudovector.  */
-- 
2.17.1


[-- Attachment #3: marker24.diff --]
[-- Type: text/x-patch, Size: 1590 bytes --]

diff --git a/src/alloc.c b/src/alloc.c
index a0639fd577..cbeb51bbc9 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -638,13 +638,23 @@ buffer_memory_full (ptrdiff_t nbytes)
 /* LISP_ALIGNMENT is the alignment of Lisp objects.  It must be at
    least GCALIGNMENT so that pointers can be tagged.  It also must be
    at least as strict as the alignment of all the C types used to
-   implement Lisp objects; since pseudovectors can contain any C type,
-   this is max_align_t.  On recent GNU/Linux x86 and x86-64 this can
-   often waste up to 8 bytes, since alignof (max_align_t) is 16 but
-   typical vectors need only an alignment of 8.  However, it is not
-   worth the hassle to avoid this waste.  */
-enum { LISP_ALIGNMENT = alignof (union { max_align_t x;
-					 GCALIGNED_UNION_MEMBER }) };
+   implement Lisp objects.  This union contains all the C types whose
+   alignment contributes to LISP_ALIGNMENT.  This is not an exhaustive
+   list of the types, just enough so that the answer works on all
+   practical Emacs targets.  This union does not contain max_align_t,
+   because with recent GCC on x86 that has an alignment of 16, but
+   Emacs does not use any types requiring an alignment more than 8.
+   Emacs modules must respect the alignment limit here.  */
+union Lisp_kitchen_sink
+{
+  double d;
+  intmax_t i;
+  uintmax_t u;
+  void (*f) (void);
+  void *p;
+  GCALIGNED_UNION_MEMBER
+};
+enum { LISP_ALIGNMENT = alignof (union Lisp_kitchen_sink) };
 verify (LISP_ALIGNMENT % GCALIGNMENT == 0);
 
 /* True if malloc (N) is known to return storage suitably aligned for