Link refcounting redesign to fix problems. Started getting rid of

client/command dependencies from threading code.

--HG--
branch : HEAD

diff --git a/src/imap/cmd-thread.c b/src/imap/cmd-thread.c
index 3cfd75cacec7a63a4cd61df6c5af6dd433762b75..3a1f00695a9da3281f99abe968b5eb2af50ffecf 100644 (file)
--- a/src/imap/cmd-thread.c
+++ b/src/imap/cmd-thread.c
@@ -63,7 +63,8 @@ bool cmd_thread(struct client_command_context *cmd)
        if (ret <= 0)
                return ret < 0;
 
-       ret = imap_thread(cmd, sargs, threading);
+       ret = imap_thread(client->mailbox, cmd->uid, client->output,
+                         sargs, threading);
        mail_search_args_unref(&sargs);
        if (ret < 0) {
                client_send_storage_error(cmd,

diff --git a/src/imap/imap-thread-finish.c b/src/imap/imap-thread-finish.c
index f10f05821c42e5dc673e115e589e3b2d4ce82105..c66ed0281540362a561b40824a99eddff0089e29 100644 (file)
--- a/src/imap/imap-thread-finish.c
+++ b/src/imap/imap-thread-finish.c
@@ -250,8 +250,8 @@ static int gather_base_subjects(struct thread_finish_context *ctx)
                if (!mail_set_uid(ctx->tmp_mail, node->uid)) {
                        /* the UID should have existed. we would have rebuild
                           the thread tree otherwise. */
-                       mail_hash_transaction_set_corrupted(
-                               ctx->hash_trans, "Found expunged UID");
+                       mail_hash_transaction_set_corrupted(ctx->hash_trans,
+                               "Found expunged UID");
                        ret = -1;
                        break;
                }
@@ -570,41 +570,36 @@ static void mail_thread_create_shadows(struct thread_finish_context *ctx,
 {
        struct mail_thread_node *node, *parent;
        struct mail_thread_root_node root;
-       ARRAY_TYPE(uint32_t) free_indexes;
-       const uint32_t *indexes;
-       unsigned int i, count;
-       uint32_t idx, parent_idx;
+       uint8_t *referenced;
+       uint32_t idx, i, j, parent_idx, alloc_size, max;
 
        memset(&root, 0, sizeof(root));
-       i_array_init(&free_indexes, 128);
+       alloc_size = record_count/8 + 1;
+       referenced = i_new(uint8_t, alloc_size);
        for (idx = 1; idx < record_count; idx++) {
                node = mail_hash_lookup_idx(ctx->hash_trans, idx);
                if (MAIL_HASH_RECORD_IS_DELETED(&node->rec))
                        continue;
 
-               if (thread_node_is_root(node)) {
-                       /* node is a duplicate root, free it later */
-                       array_append(&free_indexes, &idx, 1);
-                       continue;
-               }
-               parent = node->parent_idx == 0 ? NULL :
-                       mail_hash_lookup_idx(ctx->hash_trans, node->parent_idx);
-               if (thread_node_is_root(parent)) {
-                       if (parent != NULL) {
-                               /* parent is a duplicate root. replace it with
-                                  the real root. */
-                               node->parent_idx = 0;
-                               mail_hash_update(ctx->hash_trans, idx);
-                       }
+               if (node->parent_idx == 0) {
                        root.node.idx = idx;
                        root.node.uid = node->uid;
                        root.dummy = !node->exists;
                        array_append(&ctx->roots, &root, 1);
-               } else if (node->exists) {
+                       continue;
+               } else {
+                       /* @UNSAFE */
+                       referenced[node->parent_idx / 8] |=
+                               1 << (node->parent_idx % 8);
+               }
+
+               if (node->exists) {
                        /* Find the node's first non-dummy parent and add the
                           node as its child. If there are no non-dummy
                           parents, add it as the highest dummy's child. */
                        parent_idx = node->parent_idx;
+                       parent = mail_hash_lookup_idx(ctx->hash_trans,
+                                                     parent_idx);
                        while (!parent->exists && parent->parent_idx != 0) {
                                parent_idx = parent->parent_idx;
                                parent = mail_hash_lookup_idx(ctx->hash_trans,
@@ -614,14 +609,23 @@ static void mail_thread_create_shadows(struct thread_finish_context *ctx,
                }
        }
 
-       /* remove records from hash that have been marked as deleted */
-       indexes = array_get(&free_indexes, &count);
-       for (i = 0; i < count; i++) {
-               node = mail_hash_lookup_idx(ctx->hash_trans, indexes[i]);
-               mail_hash_remove(ctx->hash_trans, indexes[i],
-                                node->msgid_crc32);
+       /* remove unneeded records from hash */
+       for (i = 1; i < alloc_size; i++) {
+               if (referenced[i] == 0xff)
+                       continue;
+
+               max = i+1 < alloc_size ? 8 : (record_count % 8);
+               for (j = 0; j < max; j++) {
+                       if ((referenced[i] & (1 << j)) != 0)
+                               continue;
+
+                       idx = i*8 + j;
+                       node = mail_hash_lookup_idx(ctx->hash_trans, idx);
+                       mail_hash_remove(ctx->hash_trans, idx,
+                                        node->msgid_crc32);
+               }
        }
-       array_free(&free_indexes);
+       i_free(referenced);
 }
 
 int mail_thread_finish(struct mail *tmp_mail,

diff --git a/src/imap/imap-thread-links.c b/src/imap/imap-thread-links.c
index 4c743b07db6e0238d1912d0daaa90c36e50d10d0..7056ca68f5f046e35b84a5b19baa4a0cbdbf9ab5 100644 (file)
--- a/src/imap/imap-thread-links.c
+++ b/src/imap/imap-thread-links.c
@@ -94,13 +94,13 @@ static bool thread_node_has_ancestor(struct thread_context *ctx,
 static void thread_link_reference(struct thread_context *ctx,
                                  uint32_t parent_idx, uint32_t child_idx)
 {
-       struct mail_thread_node *node, *parent, *child, *orig_parent;
+       struct mail_thread_node *node, *parent, *child;
        uint32_t idx;
 
        parent = mail_hash_lookup_idx(ctx->hash_trans, parent_idx);
        child = mail_hash_lookup_idx(ctx->hash_trans, child_idx);
 
-       parent->link_refcount++;
+       child->link_refcount++;
        mail_hash_update(ctx->hash_trans, parent_idx);
 
        if (thread_node_has_ancestor(ctx, parent, child)) {
@@ -130,7 +130,7 @@ static void thread_link_reference(struct thread_context *ctx,
                                break;
                        }
                        node = mail_hash_lookup_idx(ctx->hash_trans, idx);
-                       node->unref_rebuilds = TRUE;
+                       node->parent_unref_rebuilds = TRUE;
                        mail_hash_update(ctx->hash_trans, idx);
                } while (node != child);
                return;
@@ -140,11 +140,9 @@ static void thread_link_reference(struct thread_context *ctx,
        }
 
        /* Set parent_node as child_node's parent */
-       orig_parent = child->parent_idx == 0 ? NULL :
-               mail_hash_lookup_idx(ctx->hash_trans, child->parent_idx);
-       if (thread_node_is_root(orig_parent)) {
+       if (child->parent_idx == 0)
                child->parent_idx = parent_idx;
-       } else {
+       else {
                /* Conflicting parent already exists, keep the original */
                if (child->exists) {
                        /* If this message gets expunged,
@@ -156,13 +154,13 @@ static void thread_link_reference(struct thread_context *ctx,
                           that reference gets dropped, the parent is changed.
                           We could catch this in one of several ways:
 
-                           a) Parent node gets unreferenced
-                           b) This node gets unreferenced
+                           a) Link to parent node gets unreferenced
+                           b) Link to this node gets unreferenced
                            c) Any of the child nodes gets expunged
 
                           b) is probably the least likely to happen,
                           so use it */
-                       child->unref_rebuilds = TRUE;
+                       child->parent_unref_rebuilds = TRUE;
                }
        }
        mail_hash_update(ctx->hash_trans, child_idx);
@@ -263,7 +261,7 @@ int mail_thread_add(struct thread_context *ctx, struct mail *mail)
        old_parent = node->parent_idx == 0 ? NULL :
                mail_hash_lookup_idx(ctx->hash_trans, node->parent_idx);
 
-       if (!thread_node_is_root(old_parent) &&
+       if (old_parent != NULL &&
            (parent == NULL || old_parent->parent_idx != parent_idx)) {
                /* conflicting parent, remove it. */
                node->parent_idx = 0;
@@ -308,11 +306,13 @@ mail_thread_node_lookup(struct thread_context *ctx, uint32_t uid,
        return TRUE;
 }
 
-static bool thread_unref_msgid(struct thread_context *ctx, const char *msgid)
+static bool
+thread_unref_msgid(struct thread_context *ctx, uint32_t child_idx,
+                  const char *msgid, uint32_t *parent_idx_r)
 {
        struct msgid_search_key key;
-       struct mail_thread_node *node;
-       uint32_t idx;
+       struct mail_thread_node *parent, *child;
+       uint32_t parent_idx;
 
        key.msgid = msgid;
        key.msgid_crc32 = crc32_str_nonzero(msgid);
@@ -320,8 +320,8 @@ static bool thread_unref_msgid(struct thread_context *ctx, const char *msgid)
        ctx->cmp_match_count = 0;
        ctx->cmp_last_idx = 0;
 
-       node = mail_hash_lookup(ctx->hash_trans, &key, &idx);
-       if (node == NULL) {
+       parent = mail_hash_lookup(ctx->hash_trans, &key, &parent_idx);
+       if (parent == NULL) {
                if (ctx->cmp_match_count != 1 || ctx->failed) {
                        /* couldn't find the message-id */
                        return FALSE;
@@ -329,25 +329,33 @@ static bool thread_unref_msgid(struct thread_context *ctx, const char *msgid)
 
                /* there's only one key with this crc32 value, so it
                   must be what we're looking for */
-               idx = ctx->cmp_last_idx;
-               node = mail_hash_lookup_idx(ctx->hash_trans, idx);
+               parent_idx = ctx->cmp_last_idx;
+               parent = mail_hash_lookup_idx(ctx->hash_trans, parent_idx);
        }
-       if (node->unref_rebuilds)
+       if (parent->parent_unref_rebuilds)
                return FALSE;
 
-       node->link_refcount--;
-       if (!node->exists && node->link_refcount == 0) {
-               /* we turned into a root node */
-               node->parent_idx = 0;
+       child = mail_hash_lookup_idx(ctx->hash_trans, child_idx);
+       if (child->link_refcount == 0) {
+               mail_hash_transaction_set_corrupted(ctx->hash_trans,
+                                                   "unexpected refcount=0");
+               return FALSE;
        }
-       mail_hash_update(ctx->hash_trans, idx);
+       child->link_refcount--;
+       if (child->link_refcount == 0) {
+               /* we don't have a root anymore */
+               child->parent_idx = 0;
+       }
+       mail_hash_update(ctx->hash_trans, child_idx);
+       *parent_idx_r = parent_idx;
        return TRUE;
 }
 
 static bool
-thread_unref_links(struct thread_context *ctx, const char *references,
-                  bool *valid_r)
+thread_unref_links(struct thread_context *ctx, uint32_t child_idx,
+                  const char *references, bool *valid_r)
 {
+       uint32_t parent_idx;
        const char *msgid;
 
        /* tmp_mail may be changed below, so we have to duplicate the
@@ -357,8 +365,9 @@ thread_unref_links(struct thread_context *ctx, const char *references,
 
        while ((msgid = message_id_get_next(&references)) != NULL) {
                *valid_r = TRUE;
-               if (!thread_unref_msgid(ctx, msgid))
+               if (!thread_unref_msgid(ctx, child_idx, msgid, &parent_idx))
                        return FALSE;
+               child_idx = parent_idx;
        }
        return TRUE;
 }
@@ -368,7 +377,7 @@ int mail_thread_remove(struct thread_context *ctx, uint32_t uid)
        struct mail_hash_header *hdr;
        struct mail_thread_node *node;
        const char *references, *in_reply_to;
-       uint32_t idx;
+       uint32_t idx, parent_idx;
        bool have_refs;
 
        if (!mail_thread_node_lookup(ctx, uid, &idx, &node))
@@ -380,7 +389,7 @@ int mail_thread_remove(struct thread_context *ctx, uint32_t uid)
                                  &references) < 0)
                return -1;
 
-       if (!thread_unref_links(ctx, references, &have_refs))
+       if (!thread_unref_links(ctx, idx, references, &have_refs))
                return 0;
        if (!have_refs) {
                /* no valid IDs in References:, use In-Reply-To: instead */
@@ -389,7 +398,8 @@ int mail_thread_remove(struct thread_context *ctx, uint32_t uid)
                        return -1;
                in_reply_to = message_id_get_next(&in_reply_to);
                if (in_reply_to != NULL) {
-                       if (!thread_unref_msgid(ctx, in_reply_to))
+                       if (!thread_unref_msgid(ctx, idx, in_reply_to,
+                                               &parent_idx))
                                return 0;
                }
        }
@@ -399,10 +409,6 @@ int mail_thread_remove(struct thread_context *ctx, uint32_t uid)
 
        node->uid = 0;
        node->exists = FALSE;
-       if (node->link_refcount == 0) {
-               /* we turned into a root node */
-               node->parent_idx = 0;
-       }
        mail_hash_update(ctx->hash_trans, idx);
 
        hdr = mail_hash_get_header(ctx->hash_trans);

diff --git a/src/imap/imap-thread-private.h b/src/imap/imap-thread-private.h
index 93ac2b9cd849d372d0c0c24934fb5c63a60e78f9..64764ef68deccf9d13d82893e758b5665dac27ae 100644 (file)
--- a/src/imap/imap-thread-private.h
+++ b/src/imap/imap-thread-private.h
@@ -28,7 +28,7 @@ struct mail_thread_node {
 
        uint32_t link_refcount:29;
        uint32_t expunge_rebuilds:1;
-       uint32_t unref_rebuilds:1;
+       uint32_t parent_unref_rebuilds:1;
        uint32_t exists:1;
 };
 
@@ -50,15 +50,6 @@ struct thread_context {
        unsigned int syncing:1;
 };
 
-static inline bool thread_node_is_root(const struct mail_thread_node *node)
-{
-       if (node == NULL)
-               return TRUE;
-
-       /* check also if expunging had changed this node to a root node */
-       return !node->exists && node->link_refcount == 0;
-}
-
 static inline uint32_t crc32_str_nonzero(const char *str)
 {
        uint32_t value = crc32_str(str);

diff --git a/src/imap/imap-thread.c b/src/imap/imap-thread.c
index 171e04f2fa60798abecab4e06204a334e2038911..b5b90924de4fd6f72c5b64dfd8f4d52632fc328a 100644 (file)
--- a/src/imap/imap-thread.c
+++ b/src/imap/imap-thread.c
@@ -24,15 +24,16 @@
 #define APPROX_MSGID_SIZE 45
 
 struct imap_thread_context {
+       struct mailbox *box;
+       struct ostream *output;
        struct thread_context thread_ctx;
-       struct client_command_context *cmd;
        struct mailbox_transaction_context *t;
        enum mail_thread_type thread_type;
 
        struct mail_search_context *search;
        struct mail_search_arg tmp_search_arg;
 
-       unsigned int id_is_uid:1;
+       bool id_is_uid;
 };
 
 struct imap_thread_mailbox {
@@ -188,7 +189,6 @@ imap_thread_try_use_hash(struct imap_thread_context *ctx,
                         const struct mailbox_status *status, bool reset,
                         struct mail_search_args *search_args)
 {
-       struct mailbox *box = ctx->cmd->client->mailbox;
        const struct mail_hash_header *hdr;
        struct mail_hash_transaction *hash_trans;
        uint32_t last_seq, last_uid, seq1, seq2;
@@ -262,7 +262,7 @@ again:
                mail_hash_reset(hash_trans);
        } else if (hdr->message_count > 0) {
                /* non-empty hash. add only the new messages in there. */
-               mailbox_get_seq_range(box, 1, hdr->last_uid, &seq1, &seq2);
+               mailbox_get_seq_range(ctx->box, 1, hdr->last_uid, &seq1, &seq2);
 
                if (seq2 != hdr->message_count ||
                    hdr->uid_validity != status->uidvalidity) {
@@ -274,7 +274,7 @@ again:
                        struct mail_search_arg *arg = &ctx->tmp_search_arg;
 
                        arg->type = SEARCH_SEQSET;
-                       p_array_init(&arg->value.seqset, ctx->cmd->pool, 1);
+                       p_array_init(&arg->value.seqset, search_args->pool, 1);
                        if (seq2 == last_seq) {
                                /* no need to update the index,
                                   search nothing */
@@ -312,23 +312,22 @@ again:
 }
 
 static void
-imap_thread_context_init(struct imap_thread_mailbox *tbox,
-                        struct imap_thread_context *ctx,
+imap_thread_context_init(struct imap_thread_context *ctx,
                         struct mail_search_args *search_args, bool reset)
 {
-       struct mailbox *box = ctx->cmd->client->mailbox;
+       struct imap_thread_mailbox *tbox = IMAP_THREAD_CONTEXT(ctx->box);
        struct mail_hash *hash = NULL;
        struct mailbox_status status;
        const struct mail_hash_header *hdr;
        unsigned int count;
 
-       mailbox_get_status(box, STATUS_MESSAGES | STATUS_UIDNEXT, &status);
+       mailbox_get_status(ctx->box, STATUS_MESSAGES | STATUS_UIDNEXT, &status);
        if (imap_thread_try_use_hash(ctx, tbox->hash, &status,
                                     reset, search_args))
                hash = tbox->hash;
        else {
                /* fallback to using in-memory hash */
-               struct index_mailbox *ibox = (struct index_mailbox *)box;
+               struct index_mailbox *ibox = (struct index_mailbox *)ctx->box;
 
                hash = mail_hash_alloc(ibox->index, NULL,
                                       sizeof(struct mail_thread_node),
@@ -346,7 +345,7 @@ imap_thread_context_init(struct imap_thread_mailbox *tbox,
        ctx->thread_ctx.hash = hash;
 
        /* initialize searching */
-       ctx->t = mailbox_transaction_begin(box, 0);
+       ctx->t = mailbox_transaction_begin(ctx->box, 0);
        ctx->search = mailbox_search_init(ctx->t, search_args, NULL);
        ctx->thread_ctx.tmp_mail = mail_alloc(ctx->t, 0, NULL);
 
@@ -427,19 +426,18 @@ static int imap_thread_run(struct imap_thread_context *ctx)
 
        if (mail_thread_finish(ctx->thread_ctx.tmp_mail,
                               ctx->thread_ctx.hash_trans,
-                              ctx->thread_type, ctx->cmd->client->output,
-                              ctx->cmd->uid) < 0) {
+                              ctx->thread_type, ctx->output,
+                              ctx->id_is_uid) < 0) {
                mail_storage_set_internal_error(box->storage);
                return -1;
        }
        return 0;
 }
 
-int imap_thread(struct client_command_context *cmd,
-               struct mail_search_args *args, enum mail_thread_type type)
+int imap_thread(struct mailbox *box, bool id_is_uid, struct ostream *output,
+               struct mail_search_args *args, enum mail_thread_type type)
 {
-       struct imap_thread_mailbox *tbox =
-               IMAP_THREAD_CONTEXT(cmd->client->mailbox);
+       struct imap_thread_mailbox *tbox = IMAP_THREAD_CONTEXT(box);
        struct imap_thread_context *ctx;
        int ret, try;
 
@@ -451,9 +449,11 @@ int imap_thread(struct client_command_context *cmd,
        tbox->ctx = &ctx->thread_ctx;
 
        for (try = 0; try < 2; try++) {
+               ctx->box = box;
+               ctx->output = output;
+               ctx->id_is_uid = id_is_uid;
                ctx->thread_type = type;
-               ctx->cmd = cmd;
-               imap_thread_context_init(tbox, ctx, args, try == 1);
+               imap_thread_context_init(ctx, args, try == 1);
                ret = imap_thread_run(ctx);
                if (imap_thread_finish(tbox, ctx) < 0)
                        ret = -1;

diff --git a/src/imap/imap-thread.h b/src/imap/imap-thread.h
index ec6cacde384a73e2a576fa3090e70992cd3ce51e..0a1f4530adcab12fbc4a5d0e8325de59d39877c5 100644 (file)
--- a/src/imap/imap-thread.h
+++ b/src/imap/imap-thread.h
@@ -8,8 +8,8 @@ enum mail_thread_type {
        MAIL_THREAD_REFERENCES2
 };
 
-int imap_thread(struct client_command_context *cmd,
-               struct mail_search_args *args, enum mail_thread_type type);
+int imap_thread(struct mailbox *box, bool id_is_uid, struct ostream *output,
+               struct mail_search_args *args, enum mail_thread_type type);
 
 void imap_thread_init(void);
 void imap_thread_deinit(void);