[Emacs-diffs] trunk r114448: Optimize glyph row clearing and copying routines.

[Dmitry Antipov]

------------------------------------------------------------
revno: 114448
revision-id: address@hidden
parent: address@hidden
committer: Dmitry Antipov <address@hidden>
branch nick: trunk
timestamp: Tue 2013-09-24 09:42:30 +0400
message:
  Optimize glyph row clearing and copying routines.
  * dispextern.h (struct glyph_row): Change layout of struct
  glyph_row to help copy_row_except_pointers.  Adjust comment.
  * dispnew.c (null_row): Remove.
  (clear_glyph_row): Use offsetof and memset to find and clear
  just the members that need clearing.  Adjust comment.
  (copy_row_except_pointers): Likewise for copying.
modified:
  src/ChangeLog                  changelog-20091113204419-o5vbwnq5f7feedwu-1438
  src/dispextern.h               
dispextern.h-20091113204419-o5vbwnq5f7feedwu-218
  src/dispnew.c                  dispnew.c-20091113204419-o5vbwnq5f7feedwu-258

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2013-09-24 04:28:06 +0000
+++ b/src/ChangeLog     2013-09-24 05:42:30 +0000
@@ -1,3 +1,13 @@
+2013-09-24  Dmitry Antipov  <address@hidden>
+
+       Optimize glyph row clearing and copying routines.
+       * dispextern.h (struct glyph_row): Change layout of struct
+       glyph_row to help copy_row_except_pointers.  Adjust comment.
+       * dispnew.c (null_row): Remove.
+       (clear_glyph_row): Use offsetof and memset to find and clear
+       just the members that need clearing.  Adjust comment.
+       (copy_row_except_pointers): Likewise for copying.
+
 2013-09-24  Paul Eggert  <address@hidden>
 
        Some minor cleanups of recently-added bool vector code.

=== modified file 'src/dispextern.h'
--- a/src/dispextern.h  2013-09-23 13:46:20 +0000
+++ b/src/dispextern.h  2013-09-24 05:42:30 +0000
@@ -792,7 +792,10 @@
    Rows in window matrices on frames having no frame matrices point to
    glyphs allocated from the heap via xmalloc;
    glyphs[LEFT_MARGIN_AREA] is the start address of the allocated
-   glyph structure array.  */
+   glyph structure array.
+
+   NOTE: layout of first four members of this structure is important,
+   see clear_glyph_row and copy_row_except_pointers to check why.  */
 
 struct glyph_row
 {
@@ -812,8 +815,13 @@
      removed some day, so don't use it in new code.  */
   struct glyph *glyphs[1 + LAST_AREA];
 
-  /* Number of glyphs actually filled in areas.  */
-  short used[LAST_AREA];
+  /* Number of glyphs actually filled in areas.  This could have size
+     LAST_AREA, but it's 1 + LAST_AREA to simplify offset calculations.  */
+  short used[1 + LAST_AREA];
+
+  /* Hash code.  This hash code is available as soon as the row
+     is constructed, i.e. after a call to display_line.  */
+  unsigned hash;
 
   /* Window-relative x and y-position of the top-left corner of this
      row.  If y < 0, this means that eabs (y) pixels of the row are
@@ -846,10 +854,6 @@
      in last row when checking if row is fully visible.  */
   int extra_line_spacing;
 
-  /* Hash code.  This hash code is available as soon as the row
-     is constructed, i.e. after a call to display_line.  */
-  unsigned hash;
-
   /* First position in this row.  This is the text position, including
      overlay position information etc, where the display of this row
      started, and can thus be less than the position of the first

=== modified file 'src/dispnew.c'
--- a/src/dispnew.c     2013-09-23 09:50:47 +0000
+++ b/src/dispnew.c     2013-09-24 05:42:30 +0000
@@ -832,41 +832,17 @@
       See dispextern.h for an overall explanation of glyph rows.
  ***********************************************************************/
 
-/* Clear glyph row ROW.  Do it in a way that makes it robust against
-   changes in the glyph_row structure, i.e. addition or removal of
-   structure members.  */
-
-static struct glyph_row null_row;
+/* Clear glyph row ROW.  NOTE: this code relies on the current
+   layout of `glyphs' and `used' fields of `struct glyph_row'.  */
 
 void
 clear_glyph_row (struct glyph_row *row)
 {
-  struct glyph *p[1 + LAST_AREA];
-
-  /* Save pointers.  */
-  p[LEFT_MARGIN_AREA] = row->glyphs[LEFT_MARGIN_AREA];
-  p[TEXT_AREA] = row->glyphs[TEXT_AREA];
-  p[RIGHT_MARGIN_AREA] = row->glyphs[RIGHT_MARGIN_AREA];
-  p[LAST_AREA] = row->glyphs[LAST_AREA];
-
-  /* Clear.  */
-  *row = null_row;
-
-  /* Restore pointers.  */
-  row->glyphs[LEFT_MARGIN_AREA] = p[LEFT_MARGIN_AREA];
-  row->glyphs[TEXT_AREA] = p[TEXT_AREA];
-  row->glyphs[RIGHT_MARGIN_AREA] = p[RIGHT_MARGIN_AREA];
-  row->glyphs[LAST_AREA] = p[LAST_AREA];
-
-#if 0 /* At some point, some bit-fields of struct glyph were not set,
-        which made glyphs unequal when compared with GLYPH_EQUAL_P.
-        Redisplay outputs such glyphs, and flickering effects were
-        the result.  This also depended on the contents of memory
-        returned by xmalloc.  If flickering happens again, activate
-        the code below.  If the flickering is gone with that, chances
-        are that the flickering has the same reason as here.  */
-  memset (p[0], 0, (char *) p[LAST_AREA] - (char *) p[0]);
-#endif
+  const size_t off = offsetof (struct glyph_row, used);
+
+  eassert (off == sizeof row->glyphs);
+  /* Zero everything except pointers in `glyphs'.  */
+  memset ((char *) row + off, 0, sizeof *row - off);
 }
 
 
@@ -1005,29 +981,18 @@
 }
 
 
-/* Copy glyph row structure FROM to glyph row structure TO, except
-   that glyph pointers, the `used' counts, and the hash values in the
-   structures are left unchanged.  */
+/* Copy glyph row structure FROM to glyph row structure TO, except that
+   glyph pointers, the `used' counts, and the hash values in the structures
+   are left unchanged.  NOTE: this code relies on the current layout of
+   `glyphs', `used', `hash' and `x' fields of `struct glyph_row'.  */
 
 static void
 copy_row_except_pointers (struct glyph_row *to, struct glyph_row *from)
 {
-  struct glyph *pointers[1 + LAST_AREA];
-  short used[LAST_AREA];
-  unsigned hashval;
-
-  /* Save glyph pointers of TO.  */
-  memcpy (pointers, to->glyphs, sizeof to->glyphs);
-  memcpy (used, to->used, sizeof to->used);
-  hashval = to->hash;
-
-  /* Do a structure assignment.  */
-  *to = *from;
-
-  /* Restore original pointers of TO.  */
-  memcpy (to->glyphs, pointers, sizeof to->glyphs);
-  memcpy (to->used, used, sizeof to->used);
-  to->hash = hashval;
+  const size_t off = offsetof (struct glyph_row, x);
+
+  eassert (off == sizeof to->glyphs + sizeof to->used + sizeof to->hash);
+  memcpy ((char *) to + off, (char *) from + off, sizeof *to - off);
 }