]> diplodocus.org Git - nmh/commitdiff
sbr/folder_read.c: Don't malloc() once per message.
authorRalph Corderoy <ralph@inputplus.co.uk>
Fri, 28 Apr 2017 16:12:31 +0000 (17:12 +0100)
committerRalph Corderoy <ralph@inputplus.co.uk>
Fri, 28 Apr 2017 16:12:31 +0000 (17:12 +0100)
Instead of struct msgs having a pointer to a malloc'd array of pointers,
each to a malloc'd struct bvector, 1+N, it now has a pointer to a
malloc'd array of struct bvector;  one malloc for all of them.  This
avoids the large number of calls to malloc() and free() that's linear
with the size of the folder.

But there are some downsides.  In order to step through an array of
struct bvector, code outside of sbr/vector.c needs to know the struct's
size.  The simplest way to do this is to make the struct's definition
public, with a comment that access should be through vector.c.

New functions are needed to initialise the content of an already
allocated bvector, and to finish with its content prior to deallocation.
bvector_create() and bvector_free() now also use these new functions.

Before, it was the array of pointers to bvector that would be realloc'd.
That doesn't work for the array of bvectors as they may contain pointers
to within themselves.  The solution is to malloc a new array and
bvector_copy() the ones to keep across, as folder_realloc() now does.
The other half of its logic that coped with growth at the end of the
array, has been deleted.  Also deleted, is the code to clear the
bvectors before and after the old ones as they start in that state.

h/mh.h
sbr/folder_free.c
sbr/folder_read.c
sbr/folder_realloc.c
sbr/vector.c

diff --git a/h/mh.h b/h/mh.h
index 0a62f58..8829eb4 100644 (file)
--- a/h/mh.h
+++ b/h/mh.h
@@ -187,14 +187,22 @@ extern struct swit anoyes[];      /* standard yes/no switches */
 
 #define        MBITS "\020\01EXISTS\02SELECTED\03NEW\04UNSEEN"
 
 
 #define        MBITS "\020\01EXISTS\02SELECTED\03NEW\04UNSEEN"
 
-/*
- * type for holding the sequence set of a message
- */
+/* A vector of bits for tracking the sequence membership of a single
+ * message.  Do not access the struct members; use vector.c.
+ * Do not move or copy this struct as it may contain a pointer to
+ * itself;  use bvector_copy(). */
+struct bvector {
+    unsigned long *bits;
+    size_t maxsize;
+    unsigned long tiny[2];   /* Default fixed-size storage for bits. */
+};
 typedef struct bvector *bvector_t;
 
 bvector_t bvector_create (void);
 typedef struct bvector *bvector_t;
 
 bvector_t bvector_create (void);
+void bvector_init(struct bvector *bv);
 void bvector_copy (bvector_t, bvector_t);
 void bvector_free (bvector_t);
 void bvector_copy (bvector_t, bvector_t);
 void bvector_free (bvector_t);
+void bvector_fini(struct bvector *bv);
 void bvector_clear (bvector_t, size_t);
 void bvector_clear_all (bvector_t);
 void bvector_set (bvector_t, size_t);
 void bvector_clear (bvector_t, size_t);
 void bvector_clear_all (bvector_t);
 void bvector_set (bvector_t, size_t);
@@ -263,7 +271,7 @@ struct msgs {
      * in a particular sequence.
      */
     size_t num_msgstats;
      * in a particular sequence.
      */
     size_t num_msgstats;
-    bvector_t *msgstats;       /* msg status */
+    struct bvector *msgstats;  /* msg status */
 
     /*
      * A FILE handle containing an open filehandle for the sequence file
 
     /*
      * A FILE handle containing an open filehandle for the sequence file
@@ -289,7 +297,7 @@ struct msgs {
 /*
  * macros for message and sequence manipulation
  */
 /*
  * macros for message and sequence manipulation
  */
-#define msgstat(mp,n) (mp)->msgstats[(n) - mp->lowoff]
+#define msgstat(mp,n) ((mp)->msgstats + (n) - mp->lowoff)
 #define clear_msg_flags(mp,msgnum)   bvector_clear_all (msgstat(mp, msgnum))
 #define copy_msg_flags(mp,i,j)       bvector_copy (msgstat(mp,i), msgstat(mp,j))
 #define get_msg_flags(mp,ptr,msgnum) bvector_copy (ptr, msgstat(mp, msgnum))
 #define clear_msg_flags(mp,msgnum)   bvector_clear_all (msgstat(mp, msgnum))
 #define copy_msg_flags(mp,i,j)       bvector_copy (msgstat(mp,i), msgstat(mp,j))
 #define get_msg_flags(mp,ptr,msgnum) bvector_copy (ptr, msgstat(mp, msgnum))
index 0b00819..be2771f 100644 (file)
@@ -13,7 +13,7 @@ void
 folder_free (struct msgs *mp)
 {
     size_t i;
 folder_free (struct msgs *mp)
 {
     size_t i;
-    bvector_t *v;
+    struct bvector *v;
 
     if (!mp)
        return;
 
     if (!mp)
        return;
@@ -26,7 +26,7 @@ folder_free (struct msgs *mp)
     svector_free (mp->msgattrs);
 
     for (i = 0, v = mp->msgstats; i < mp->num_msgstats; ++i, ++v) {
     svector_free (mp->msgattrs);
 
     for (i = 0, v = mp->msgstats; i < mp->num_msgstats; ++i, ++v) {
-       bvector_free (*v);
+       bvector_fini(v);
     }
     free (mp->msgstats);
 
     }
     free (mp->msgstats);
 
index e157dd2..eab1e1f 100644 (file)
@@ -27,7 +27,7 @@ folder_read (char *name, int lockflag)
     struct msgs *mp;
     struct dirent *dp;
     DIR *dd;
     struct msgs *mp;
     struct dirent *dp;
     DIR *dd;
-    bvector_t *v;
+    struct bvector *v;
     size_t i;
 
     name = m_mailpath (name);
     size_t i;
 
     name = m_mailpath (name);
@@ -136,7 +136,7 @@ folder_read (char *name, int lockflag)
     mp->num_msgstats = MSGSTATNUM (mp->lowoff, mp->hghoff);
     mp->msgstats = mh_xmalloc (MSGSTATSIZE(mp));
     for (i = 0, v = mp->msgstats; i < mp->num_msgstats; ++i, ++v) {
     mp->num_msgstats = MSGSTATNUM (mp->lowoff, mp->hghoff);
     mp->msgstats = mh_xmalloc (MSGSTATSIZE(mp));
     for (i = 0, v = mp->msgstats; i < mp->num_msgstats; ++i, ++v) {
-       *v = bvector_create ();
+        bvector_init(v);
     }
 
     mp->msgattrs = svector_create (0);
     }
 
     mp->msgattrs = svector_create (0);
index 2084e0e..8ed95e2 100644 (file)
@@ -19,6 +19,8 @@
 struct msgs *
 folder_realloc (struct msgs *mp, int lo, int hi)
 {
 struct msgs *
 folder_realloc (struct msgs *mp, int lo, int hi)
 {
+    struct bvector *tmpstats, *t;
+    size_t i;
     int msgnum;
 
     /* sanity checks */
     int msgnum;
 
     /* sanity checks */
@@ -37,70 +39,22 @@ folder_realloc (struct msgs *mp, int lo, int hi)
     if (lo == mp->lowoff && hi == mp->hghoff)
        return mp;
 
     if (lo == mp->lowoff && hi == mp->hghoff)
        return mp;
 
-    if (lo == mp->lowoff) {
-       bvector_t *v;
-       size_t old_size = mp->num_msgstats;
-       size_t new_size = hi - lo + 1;
-       size_t i;
-
-       /*
-        * We are just extending (or shrinking) the end of message
-        * status array.  So we don't have to move anything and can
-        * just realloc the message status array.
-        */
-
-       for (i = old_size, v = &mp->msgstats[old_size-1]; i > new_size;
-            --i, --v) {
-           bvector_free (*v);
-       }
-       mp->num_msgstats = MSGSTATNUM (lo, hi);
-       mp->msgstats =
-           mh_xrealloc (mp->msgstats, MSGSTATSIZE(mp));
-       for (i = old_size, v = &mp->msgstats[old_size]; i < new_size;
-            ++i, ++v) {
-           *v = bvector_create ();
-       }
-    } else {
-       /*
-        * We are changing the offset of the message status
-        * array.  So we will need to shift everything.
-        */
-       bvector_t *tmpstats, *t;
-       size_t i;
-
-       /* first allocate the new message status space */
-       mp->num_msgstats = MSGSTATNUM (lo, hi);
-       tmpstats = mh_xmalloc (MSGSTATSIZE(mp));
-       for (i = 0, t = tmpstats; i < mp->num_msgstats; ++i, ++t) {
-           *t = bvector_create ();
-       }
-
-       /* then copy messages status array with shift */
-       if (mp->nummsg > 0) {
-           for (msgnum = mp->lowmsg; msgnum <= mp->hghmsg; msgnum++)
-               bvector_copy (tmpstats[msgnum - lo], msgstat (mp, msgnum));
-       }
-       free(mp->msgstats);
-       mp->msgstats = tmpstats;
+    /* first allocate the new message status space */
+    mp->num_msgstats = MSGSTATNUM (lo, hi);
+    tmpstats = mh_xmalloc (MSGSTATSIZE(mp));
+    for (i = 0, t = tmpstats; i < mp->num_msgstats; ++i, ++t) {
+        bvector_init(t);
     }
 
     }
 
-    mp->lowoff = lo;
-    mp->hghoff = hi;
-
-    /*
-     * Clear all the flags for entries outside
-     * the current message range for this folder.
-     */
+    /* then copy messages status array with shift */
     if (mp->nummsg > 0) {
     if (mp->nummsg > 0) {
-       for (msgnum = mp->lowoff; msgnum < mp->lowmsg; msgnum++)
-           clear_msg_flags (mp, msgnum);
-       for (msgnum = mp->hghmsg + 1; msgnum <= mp->hghoff; msgnum++)
-           clear_msg_flags (mp, msgnum);
-    } else {
-       /* no messages, so clear entire range */
-       for (msgnum = mp->lowoff; msgnum <= mp->hghoff; msgnum++)
-           clear_msg_flags (mp, msgnum);
+        for (msgnum = mp->lowmsg; msgnum <= mp->hghmsg; msgnum++)
+            bvector_copy (tmpstats + msgnum - lo, msgstat (mp, msgnum));
     }
     }
+    free(mp->msgstats);
+    mp->msgstats = tmpstats;
+    mp->lowoff = lo;
+    mp->hghoff = hi;
 
     return mp;
 }
 
     return mp;
 }
index e376065..7df3618 100644 (file)
 /* The number of elements bits needs to cover bit n, measured in bytes. */
 #define BVEC_BYTES(n) (((n) / BVEC_BITS_BITS + 1) * BVEC_SIZEOF_BITS)
 
 /* The number of elements bits needs to cover bit n, measured in bytes. */
 #define BVEC_BYTES(n) (((n) / BVEC_BITS_BITS + 1) * BVEC_SIZEOF_BITS)
 
-struct bvector {
-    unsigned long *bits;
-    size_t maxsize;
-    unsigned long tiny[2];   /* Default fixed-size storage for bits. */
-};
-
 /* bvector_resize ensures the storage used for bits can cover bit
  * newsize.  It always increases the size of the storage used for bits,
  * even if newsize would have been covered by the existing storage.
 /* bvector_resize ensures the storage used for bits can cover bit
  * newsize.  It always increases the size of the storage used for bits,
  * even if newsize would have been covered by the existing storage.
@@ -67,13 +61,18 @@ bvector_create (void) {
     assert (sizeof *vec->bits <= sizeof 1ul);
 
     NEW(vec);
     assert (sizeof *vec->bits <= sizeof 1ul);
 
     NEW(vec);
-    vec->bits = vec->tiny;
-    vec->maxsize = BVEC_INIT_SIZE;
-    memset(vec->tiny, 0, sizeof vec->tiny);
+    bvector_init(vec);
 
     return vec;
 }
 
 
     return vec;
 }
 
+void bvector_init(struct bvector *bv)
+{
+    bv->bits = bv->tiny;
+    bv->maxsize = BVEC_INIT_SIZE;
+    memset(bv->tiny, 0, sizeof bv->tiny);
+}
+
 void
 bvector_copy (bvector_t dest, bvector_t src) {
     size_t bytes = BVEC_BYTES(src->maxsize);
 void
 bvector_copy (bvector_t dest, bvector_t src) {
     size_t bytes = BVEC_BYTES(src->maxsize);
@@ -87,11 +86,16 @@ bvector_copy (bvector_t dest, bvector_t src) {
 
 void
 bvector_free (bvector_t vec) {
 
 void
 bvector_free (bvector_t vec) {
-    if (vec->bits != vec->tiny)
-        free(vec->bits);
+    bvector_fini(vec);
     free (vec);
 }
 
     free (vec);
 }
 
+void bvector_fini(struct bvector *bv)
+{
+    if (bv->bits != bv->tiny)
+        free(bv->bits);
+}
+
 void
 bvector_clear (bvector_t vec, size_t n) {
     if (n < vec->maxsize)
 void
 bvector_clear (bvector_t vec, size_t n) {
     if (n < vec->maxsize)