]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
mdbox: Optimization: Don't update map UIDs' refcounts via array when it's not necessary.
authorTimo Sirainen <tss@iki.fi>
Mon, 19 Apr 2010 13:15:57 +0000 (16:15 +0300)
committerTimo Sirainen <tss@iki.fi>
Mon, 19 Apr 2010 13:15:57 +0000 (16:15 +0300)
--HG--
branch : HEAD

src/lib-storage/index/dbox-multi/mdbox-map.c
src/lib-storage/index/dbox-multi/mdbox-map.h
src/lib-storage/index/dbox-multi/mdbox-storage.c
src/lib-storage/index/dbox-multi/mdbox-sync.c
src/lib-storage/index/dbox-multi/mdbox-sync.h

index eb020561f0d0660a0f7846a94114ae96309a7852..4bb8a8eeaf559bde8ad876c357382ba29a2a7c7a 100644 (file)
@@ -409,50 +409,61 @@ void dbox_map_transaction_free(struct dbox_map_transaction_context **_ctx)
        i_free(ctx);
 }
 
-int dbox_map_update_refcounts(struct dbox_map_transaction_context *ctx,
-                             const ARRAY_TYPE(uint32_t) *map_uids, int diff)
+int dbox_map_update_refcount(struct dbox_map_transaction_context *ctx,
+                            uint32_t map_uid, int diff)
 {
        struct dbox_map *map = ctx->map;
-       const uint32_t *uidp;
-       unsigned int i, count;
        const void *data;
        uint32_t seq;
        bool expunged;
        int cur_diff;
 
-       if (ctx->trans == NULL)
+       if (unlikely(ctx->trans == NULL))
+               return -1;
+
+       if (!mail_index_lookup_seq(map->view, map_uid, &seq)) {
+               /* we can't refresh map here since view has a
+                  transaction open. */
+               dbox_map_set_corrupted(map, "refcount update lost map_uid=%u",
+                                      map_uid);
+               return -1;
+       }
+       mail_index_lookup_ext(map->view, seq, map->ref_ext_id,
+                             &data, &expunged);
+       cur_diff = data == NULL ? 0 : *((const uint16_t *)data);
+       ctx->changed = TRUE;
+       cur_diff += mail_index_atomic_inc_ext(ctx->trans, seq,
+                                             map->ref_ext_id, diff);
+       if (cur_diff < 0) {
+               dbox_map_set_corrupted(map, "map_uid=%u refcount too low",
+                                      map_uid);
+               return -1;
+       }
+       if (cur_diff >= 32768) {
+               /* we're getting close to the 64k limit. fail early
+                  to make it less likely that two processes increase
+                  the refcount enough times to cross the limit */
+               mail_storage_set_error(MAP_STORAGE(map), MAIL_ERROR_NOTPOSSIBLE,
+                       "Message has been copied too many times");
+               return -1;
+       }
+       return 0;
+}
+
+int dbox_map_update_refcounts(struct dbox_map_transaction_context *ctx,
+                             const ARRAY_TYPE(uint32_t) *map_uids, int diff)
+{
+       const uint32_t *uidp;
+       unsigned int i, count;
+
+       if (unlikely(ctx->trans == NULL))
                return -1;
 
        count = array_count(map_uids);
        for (i = 0; i < count; i++) {
                uidp = array_idx(map_uids, i);
-               if (!mail_index_lookup_seq(map->view, *uidp, &seq)) {
-                       /* we can't refresh map here since view has a
-                          transaction open. */
-                       dbox_map_set_corrupted(map,
-                               "refcount update lost map_uid=%u", *uidp);
+               if (dbox_map_update_refcount(ctx, *uidp, diff) < 0)
                        return -1;
-               }
-               mail_index_lookup_ext(map->view, seq, map->ref_ext_id,
-                                     &data, &expunged);
-               cur_diff = data == NULL ? 0 : *((const uint16_t *)data);
-               ctx->changed = TRUE;
-               cur_diff += mail_index_atomic_inc_ext(ctx->trans, seq,
-                                                     map->ref_ext_id, diff);
-               if (cur_diff < 0) {
-                       dbox_map_set_corrupted(map,
-                               "map_uid=%u refcount too low", *uidp);
-                       return -1;
-               }
-               if (cur_diff >= 32768) {
-                       /* we're getting close to the 64k limit. fail early
-                          to make it less likely that two processes increase
-                          the refcount enough times to cross the limit */
-                       mail_storage_set_error(MAP_STORAGE(map),
-                               MAIL_ERROR_NOTPOSSIBLE,
-                               "Message has been copied too many times");
-                       return -1;
-               }
        }
        return 0;
 }
index 73fa9ad593f19ec01036092a00bbb5843f9deef8..c7bbbffbb981c04beab90e6ca7d6f27c63f42c02 100644 (file)
@@ -58,6 +58,8 @@ int dbox_map_transaction_commit(struct dbox_map_transaction_context *ctx);
 void dbox_map_transaction_set_failed(struct dbox_map_transaction_context *ctx);
 void dbox_map_transaction_free(struct dbox_map_transaction_context **ctx);
 
+int dbox_map_update_refcount(struct dbox_map_transaction_context *ctx,
+                            uint32_t map_uid, int diff);
 int dbox_map_update_refcounts(struct dbox_map_transaction_context *ctx,
                              const ARRAY_TYPE(uint32_t) *map_uids, int diff);
 int dbox_map_remove_file_id(struct dbox_map *map, uint32_t file_id);
index 99ce0c93b19045754d9ab93f021338bbc527fb05..34b6a9f8311564f2da0b4d15ae8138e7751ac07d 100644 (file)
@@ -282,35 +282,27 @@ static int mdbox_mailbox_unref_mails(struct mdbox_mailbox *mbox)
 {
        struct dbox_map_transaction_context *map_trans;
        const struct mail_index_header *hdr;
-       const struct mdbox_mail_index_record *dbox_rec;
-       ARRAY_TYPE(uint32_t) map_uids;
-       const void *data;
-       bool expunged;
-       uint32_t seq;
-       int ret;
+       uint32_t seq, map_uid;
+       int ret = 0;
 
-       /* get a list of all map_uids in this mailbox */
-       i_array_init(&map_uids, 128);
+       map_trans = dbox_map_transaction_begin(mbox->storage->map, FALSE);
        hdr = mail_index_get_header(mbox->box.view);
        for (seq = 1; seq <= hdr->messages_count; seq++) {
-               mail_index_lookup_ext(mbox->box.view, seq, mbox->ext_id,
-                                     &data, &expunged);
-               dbox_rec = data;
-               if (dbox_rec == NULL) {
-                       /* no multi-mails */
+               if (mdbox_mail_lookup(mbox, mbox->box.view, seq,
+                                     &map_uid) < 0) {
+                       ret = -1;
+                       break;
+               }
+
+               if (dbox_map_update_refcount(map_trans, map_uid, -1) < 0) {
+                       ret = -1;
                        break;
                }
-               if (dbox_rec->map_uid != 0)
-                       array_append(&map_uids, &dbox_rec->map_uid, 1);
        }
 
-       /* unreference the map_uids */
-       map_trans = dbox_map_transaction_begin(mbox->storage->map, FALSE);
-       ret = dbox_map_update_refcounts(map_trans, &map_uids, -1);
        if (ret == 0)
                ret = dbox_map_transaction_commit(map_trans);
        dbox_map_transaction_free(&map_trans);
-       array_free(&map_uids);
        return ret;
 }
 
index c3b8e009dfdc1677a6be0ccfeec451b8db6504a3..d1705a000f2226df7fe4528b6bcf547ee7cdeef2 100644 (file)
@@ -55,12 +55,13 @@ static int mdbox_sync_expunge(struct mdbox_sync_context *ctx, uint32_t seq,
        if (mdbox_mail_lookup(ctx->mbox, ctx->sync_view, seq, &map_uid) < 0)
                return -1;
 
+       if (dbox_map_update_refcount(ctx->map_trans, map_uid, -1) < 0)
+               return -1;
        seq_range_array_add(&ctx->expunged_seqs, 0, seq);
-       array_append(&ctx->expunged_map_uids, &map_uid, 1);
        return 0;
 }
 
-static int mdbox_sync_add(struct mdbox_sync_context *ctx,
+static int mdbox_sync_rec(struct mdbox_sync_context *ctx,
                          const struct mail_index_sync_rec *sync_rec)
 {
        uint32_t seq, seq1, seq2;
@@ -119,30 +120,6 @@ static int dbox_sync_mark_expunges(struct mdbox_sync_context *ctx)
        return 0;
 }
 
-static int mdbox_sync_index_finish_expunges(struct mdbox_sync_context *ctx)
-{
-       struct dbox_map_transaction_context *map_trans;
-       int ret;
-
-       map_trans = dbox_map_transaction_begin(ctx->mbox->storage->map, FALSE);
-       ret = dbox_map_update_refcounts(map_trans, &ctx->expunged_map_uids, -1);
-       if (ret == 0) {
-               /* write refcount changes to map index */
-               ret = dbox_map_transaction_commit(map_trans);
-               if (ret == 0) {
-                       /* write changes to mailbox index */
-                       ret = dbox_sync_mark_expunges(ctx);
-               }
-       }
-
-       /* this finally finishes the map sync and makes it clear that the
-          map transaction was successfully finished. */
-       if (ret < 0)
-               dbox_map_transaction_set_failed(map_trans);
-       dbox_map_transaction_free(&map_trans);
-       return ret;
-}
-
 static int mdbox_sync_index(struct mdbox_sync_context *ctx)
 {
        struct mailbox *box = &ctx->mbox->box;
@@ -164,21 +141,33 @@ static int mdbox_sync_index(struct mdbox_sync_context *ctx)
                                             seq1, seq2);
        }
 
-       /* read all changes and group changes to same file_id together */
+       ctx->map_trans =
+               dbox_map_transaction_begin(ctx->mbox->storage->map, FALSE);
        i_array_init(&ctx->expunged_seqs, 64);
-       i_array_init(&ctx->expunged_map_uids, 64);
        while (mail_index_sync_next(ctx->index_sync_ctx, &sync_rec)) {
-               if ((ret = mdbox_sync_add(ctx, &sync_rec)) < 0)
+               if ((ret = mdbox_sync_rec(ctx, &sync_rec)) < 0)
                        break;
        }
-       if (ret == 0 && array_count(&ctx->expunged_seqs) > 0)
-               ret = mdbox_sync_index_finish_expunges(ctx);
+
+       if (ret == 0) {
+               /* write refcount changes to map index */
+               ret = dbox_map_transaction_commit(ctx->map_trans);
+       }
+       if (ret == 0) {
+               /* write changes to mailbox index */
+               ret = dbox_sync_mark_expunges(ctx);
+       }
+
+       /* this finally finishes the map sync and marks the map transaction
+          to be successfully finished. */
+       if (ret < 0)
+               dbox_map_transaction_set_failed(ctx->map_trans);
+       dbox_map_transaction_free(&ctx->map_trans);
 
        if (box->v.sync_notify != NULL)
                box->v.sync_notify(box, 0, 0);
 
        array_free(&ctx->expunged_seqs);
-       array_free(&ctx->expunged_map_uids);
        return ret == 0 ? 1 :
                (ctx->mbox->storage->storage.files_corrupted ? 0 : -1);
 }
index 165a383a6c3459afe6f9df6a977033f3c3369287..37d0d6e585531ded3537c5ee35edc5914f1ce2a3 100644 (file)
@@ -16,12 +16,10 @@ struct mdbox_sync_context {
         struct mail_index_sync_ctx *index_sync_ctx;
        struct mail_index_view *sync_view;
        struct mail_index_transaction *trans;
+       struct dbox_map_transaction_context *map_trans;
        enum mdbox_sync_flags flags;
 
        ARRAY_TYPE(seq_range) expunged_seqs;
-       /* list of expunged map_uids. the same map_uid may be listed more than
-          once in case message has been copied multiple times to mailbox. */
-       ARRAY_TYPE(uint32_t) expunged_map_uids;
 };
 
 int mdbox_sync_begin(struct mdbox_mailbox *mbox, enum mdbox_sync_flags flags,