From 159b2a1ede7bd581508e8704112d83efd44e0dfa Mon Sep 17 00:00:00 2001 From: Ralph Corderoy Date: Fri, 28 Apr 2017 17:12:31 +0100 Subject: [PATCH] sbr/folder_read.c: Don't malloc() once per message. 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 | 18 ++++++++--- sbr/folder_free.c | 4 +-- sbr/folder_read.c | 4 +-- sbr/folder_realloc.c | 74 +++++++++----------------------------------- sbr/vector.c | 26 +++++++++------- 5 files changed, 46 insertions(+), 80 deletions(-) diff --git a/h/mh.h b/h/mh.h index 0a62f586..8829eb4c 100644 --- 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" -/* - * 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); +void bvector_init(struct bvector *bv); 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); @@ -263,7 +271,7 @@ struct msgs { * 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 @@ -289,7 +297,7 @@ struct msgs { /* * 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)) diff --git a/sbr/folder_free.c b/sbr/folder_free.c index 0b008194..be2771f7 100644 --- a/sbr/folder_free.c +++ b/sbr/folder_free.c @@ -13,7 +13,7 @@ void folder_free (struct msgs *mp) { size_t i; - bvector_t *v; + struct bvector *v; 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) { - bvector_free (*v); + bvector_fini(v); } free (mp->msgstats); diff --git a/sbr/folder_read.c b/sbr/folder_read.c index e157dd25..eab1e1f4 100644 --- a/sbr/folder_read.c +++ b/sbr/folder_read.c @@ -27,7 +27,7 @@ folder_read (char *name, int lockflag) struct msgs *mp; struct dirent *dp; DIR *dd; - bvector_t *v; + struct bvector *v; 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) { - *v = bvector_create (); + bvector_init(v); } mp->msgattrs = svector_create (0); diff --git a/sbr/folder_realloc.c b/sbr/folder_realloc.c index 2084e0ee..8ed95e2e 100644 --- a/sbr/folder_realloc.c +++ b/sbr/folder_realloc.c @@ -19,6 +19,8 @@ struct msgs * folder_realloc (struct msgs *mp, int lo, int hi) { + struct bvector *tmpstats, *t; + size_t i; 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) { - 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) { - 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; } diff --git a/sbr/vector.c b/sbr/vector.c index e3760651..7df36187 100644 --- a/sbr/vector.c +++ b/sbr/vector.c @@ -47,12 +47,6 @@ /* 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. @@ -67,13 +61,18 @@ bvector_create (void) { 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; } +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); @@ -87,11 +86,16 @@ bvector_copy (bvector_t dest, bvector_t src) { void bvector_free (bvector_t vec) { - if (vec->bits != vec->tiny) - free(vec->bits); + bvector_fini(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) -- 2.56.0