[Emacs-diffs] master 2c8520e: Shrink pseudovectors a bit

[Paul Eggert]

branch: master
commit 2c8520e19c0fe72d046033e39953b7a0a87be24e
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    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 28ca780..abb98a9 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 0e38c61..6551549 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 66b2333..5ca7c59 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 a3bb633..ad7376a 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 52db7b4..79dce15 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 78c25f9..7e365e8 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 6bc2214..3c6dd7b 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 8b5f648..2114291 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 8ecb008..28d8d86 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 d4fc556..04de965 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 013083e..cc0b6b6 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 1849a5c..2ea8a93 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 89fc7ff..c203d4f 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.  */