[haiku-commits] BRANCH axeld-github.imap [af02a4f] in src/add-ons/mail_daemon/inbound_protocols/imap: . imap_lib
- From: axeld-github.imap <community@xxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 19 Jul 2013 00:15:33 +0200 (CEST)
added 1 changeset to branch 'refs/remotes/axeld-github/imap'
old head: 002f09437f5f468e0c52c884aa7fcdf9e37251d3
new head: af02a4f749fd72ce16b2318b4b5755f4d14aad65
overview: https://github.com/axeld/haiku/compare/002f094...af02a4f
----------------------------------------------------------------------------
af02a4f: IMAP: let mail retrieval scale better.
* We now also maintain a mail index -> UID array in IMAPMailbox.
* Instead of fetching the message entries by a fixed range of UIDs, we use the
message count, and get the entries by index.
* Likewise, in FetchHeadersCommand, we now get a list of UIDs rather than a
range. This makes it possible to only download exactly the headers we want.
* Extended FetchCommand to be able to dynamically build a sequence list from
a list of UIDs.
* Besides the suboptimal body fetching (one at a time, due to holes in the
IMAP specification), we should now be able to retrieve the messages with
pretty much optimal performance, while retaining an acceptable
responsiveness
to user requests.
[ Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> ]
----------------------------------------------------------------------------
Commit: af02a4f749fd72ce16b2318b4b5755f4d14aad65
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Thu Jul 18 21:53:17 2013 UTC
----------------------------------------------------------------------------
6 files changed, 168 insertions(+), 66 deletions(-)
.../imap/IMAPConnectionWorker.cpp | 122 +++++++++++--------
.../imap/IMAPConnectionWorker.h | 3 +
.../inbound_protocols/imap/IMAPMailbox.cpp | 29 ++++-
.../inbound_protocols/imap/IMAPMailbox.h | 10 +-
.../inbound_protocols/imap/imap_lib/Commands.cpp | 61 ++++++++--
.../inbound_protocols/imap/imap_lib/Commands.h | 9 +-
----------------------------------------------------------------------------
diff --git
a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.cpp
b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.cpp
index 34f58cc..d4c5562 100644
--- a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.cpp
+++ b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.cpp
@@ -15,6 +15,9 @@
#include "IMAPProtocol.h"
+using IMAP::MessageUIDList;
+
+
static const uint32 kMaxFetchEntries = 500;
static const uint32 kMaxDirectDownloadSize = 4096;
@@ -59,11 +62,16 @@ public:
return fWorker._MailboxFor(folder);
}
- int32 BodyFetchLimit()
+ int32 BodyFetchLimit() const
{
return fWorker.fSettings.BodyFetchLimit();
}
+ uint32 MessagesExist() const
+ {
+ return fWorker._MessagesExist();
+ }
+
status_t EnqueueCommand(WorkerCommand* command)
{
return fWorker._EnqueueCommand(command);
@@ -151,13 +159,15 @@ public:
fUID = *fEntries.begin();
fEntries.erase(fEntries.begin());
- // TODO: check nextUID if we get one
status_t status = WorkerPrivate(worker).SelectMailbox(fFolder);
if (status != B_OK)
return status;
printf("IMAP: fetch body for %lu\n", fUID);
- // TODO: combine smaller messages together for faster retrieval
+ // Since RFC3501 does not specify whether the FETCH response may
+ // alter the order of the message data items we request, we
cannot
+ // request more than a single UID at a time, or else we may not
be
+ // able to assign the data to the correct message beforehand.
IMAP::FetchCommand fetch(fUID, fUID, IMAP::kFetchBody);
fetch.SetListener(this);
@@ -195,12 +205,11 @@ private:
class FetchHeadersCommand : public SyncCommand, public IMAP::FetchListener {
public:
FetchHeadersCommand(IMAPFolder& folder, IMAPMailbox& mailbox,
- uint32 from, uint32 to, int32 bodyFetchLimit)
+ MessageUIDList& uids, int32 bodyFetchLimit)
:
fFolder(folder),
fMailbox(mailbox),
- fFrom(from),
- fTo(to),
+ fUIDs(uids),
fBodyFetchLimit(bodyFetchLimit)
{
}
@@ -209,19 +218,13 @@ public:
{
IMAP::Protocol& protocol = WorkerPrivate(worker).Protocol();
- // TODO: check nextUID if we get one
status_t status = WorkerPrivate(worker).SelectMailbox(fFolder);
if (status != B_OK)
return status;
- // TODO: this does not scale that well. Over time, the holes in
the
- // UIDs might become really large
- uint32 to = fTo;
- if (to - fFrom >= kMaxFetchEntries)
- to = fFrom + kMaxFetchEntries - 1;
+ printf("IMAP: fetch %" B_PRIuSIZE "u headers\n", fUIDs.size());
- printf("IMAP: fetch headers from %lu to %lu\n", fFrom, to);
- IMAP::FetchCommand fetch(fFrom, to,
+ IMAP::FetchCommand fetch(fUIDs, kMaxFetchEntries,
IMAP::kFetchHeader | IMAP::kFetchFlags);
fetch.SetListener(this);
@@ -229,8 +232,6 @@ public:
if (status != B_OK)
return status;
- fFrom = to + 1;
-
if (IsDone() && !fFetchBodies.empty()) {
// Enqueue command to fetch the message bodies
WorkerPrivate(worker).EnqueueCommand(new
FetchBodiesCommand(fFolder,
@@ -242,7 +243,7 @@ public:
virtual bool IsDone() const
{
- return fFrom >= fTo;
+ return fUIDs.empty();
}
virtual bool FetchData(uint32 fetchFlags, BDataIO& stream, size_t&
length)
@@ -266,10 +267,9 @@ public:
private:
IMAPFolder& fFolder;
IMAPMailbox& fMailbox;
- uint32 fFrom;
- uint32 fTo;
+ MessageUIDList fUIDs;
+ MessageUIDList fFetchBodies;
uint32 fBodyFetchLimit;
- std::vector<uint32> fFetchBodies;
entry_ref fRef;
BFile fFile;
status_t fFetchStatus;
@@ -318,14 +318,14 @@ public:
fMailbox = WorkerPrivate(worker).MailboxFor(*fFolder);
- status_t status =
WorkerPrivate(worker).SelectMailbox(*fFolder,
- fNextUID);
+ status_t status =
WorkerPrivate(worker).SelectMailbox(*fFolder);
if (status != B_OK)
return status;
- fState = FETCH_ENTRIES;
- fFirstUID = fLastUID = fFolder->LastUID() + 1;
- fMailboxEntries = 0;
+ fLastIndex = WorkerPrivate(worker).MessagesExist();
+ fFirstIndex = fMailbox->CountMessages() + 1;
+ if (fLastIndex > 0)
+ fState = FETCH_ENTRIES;
}
if (fState == FETCH_ENTRIES) {
@@ -333,44 +333,49 @@ public:
if (status != B_OK)
return status;
- // TODO: this does not scale that well. Over time, the
holes in the
- // UIDs might become really large
- uint32 from = fLastUID;
- uint32 to = fNextUID;
- if (to - from >= kMaxFetchEntries)
- to = from + kMaxFetchEntries - 1;
+ uint32 to = fLastIndex;
+ uint32 from = fFirstIndex + kMaxFetchEntries < to
+ ? fLastIndex - kMaxFetchEntries : fFirstIndex;
printf("IMAP: get entries from %lu to %lu\n", from, to);
- // TODO: we don't really need the flags at this point
at all
- IMAP::MessageEntryList entries;
- IMAP::FetchMessageEntriesCommand fetch(entries, from,
to, true);
+
+ IMAP::MessageEntryList entries;
+ IMAP::FetchMessageEntriesCommand fetch(entries, from,
to, false);
status = protocol.ProcessCommand(fetch);
if (status != B_OK)
return status;
+ std::vector<uint32> uidsToFetch;
+
// Determine how much we need to download
// TODO: also retrieve the header size, and only take
the body
- // size into account if it's below the limit
+ // size into account if it's below the limit -- that
does not
+ // seem to be possible, though
for (size_t i = 0; i < entries.size(); i++) {
printf("%10lu %8lu bytes, flags: %#lx\n",
entries[i].uid,
entries[i].size, entries[i].flags);
- fMailbox->AddMessageEntry(entries[i].uid,
entries[i].flags,
- entries[i].size);
- fTotalBytes += entries[i].size;
+ fMailbox->AddMessageEntry(from + i,
entries[i].uid,
+ entries[i].flags, entries[i].size);
+
+ if (entries[i].uid > fFolder->LastUID()) {
+ fTotalBytes += entries[i].size;
+ fUIDsToFetch.push_back(entries[i].uid);
+ }
}
- fTotalEntries += entries.size();
- fMailboxEntries += entries.size();
- fLastUID = to + 1;
+ fTotalEntries += fUIDsToFetch.size();
+ fLastIndex = from - 1;
- if (to == fNextUID) {
- if (fMailboxEntries > 0) {
+ if (from == 1) {
+ if (fUIDsToFetch.size() > 0) {
// Add pending command to fetch the
message headers
WorkerCommand* command = new
FetchHeadersCommand(*fFolder,
- *fMailbox, fFirstUID, fNextUID,
+ *fMailbox, fUIDsToFetch,
WorkerPrivate(worker).BodyFetchLimit());
if (!fFetchCommands.AddItem(command))
delete command;
+
+ fUIDsToFetch.clear();
}
fState = SELECT;
}
@@ -397,13 +402,12 @@ private:
State fState;
IMAPFolder* fFolder;
IMAPMailbox* fMailbox;
- uint32 fFirstUID;
- uint32 fLastUID;
- uint32 fNextUID;
- uint32 fMailboxEntries;
+ uint32 fFirstIndex;
+ uint32 fLastIndex;
uint64 fTotalEntries;
uint64 fTotalBytes;
WorkerCommandList fFetchCommands;
+ MessageUIDList fUIDsToFetch;
};
@@ -416,7 +420,7 @@ struct CommandDelete
};
-/*! An auto deleter similar to ObjectDeleter that called SyncCommandDone()
+/*! An auto deleter similar to ObjectDeleter that calls SyncCommandDone()
for all SyncCommands.
*/
struct CommandDeleter : BPrivate::AutoDeleter<WorkerCommand, CommandDelete>
@@ -454,6 +458,9 @@ IMAPConnectionWorker::IMAPConnectionWorker(IMAPProtocol&
owner,
{
fExistsHandler.SetListener(this);
fProtocol.AddHandler(fExistsHandler);
+
+ fExpungeHandler.SetListener(this);
+ fProtocol.AddHandler(fExpungeHandler);
}
@@ -568,9 +575,14 @@ IMAPConnectionWorker::EnqueueRetrieveMail(entry_ref& ref)
void
-IMAPConnectionWorker::MessageExistsReceived(uint32 index)
+IMAPConnectionWorker::MessageExistsReceived(uint32 count)
{
- printf("Message exists: %ld\n", index);
+ printf("Message exists: %lu\n", count);
+ fMessagesExist = count;
+
+ // TODO: We might want to trigger another check even during sync
+ // (but only one), if this isn't the result of a SELECT
+ EnqueueCheckMailboxes();
}
@@ -578,6 +590,14 @@ void
IMAPConnectionWorker::MessageExpungeReceived(uint32 index)
{
printf("Message expunge: %ld\n", index);
+ if (fSelectedBox == NULL)
+ return;
+
+ IMAPMailbox* mailbox = _MailboxFor(*fSelectedBox);
+ if (mailbox != NULL) {
+ mailbox->RemoveMessageEntry(index);
+ // TODO: remove message from folder
+ }
}
diff --git
a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.h
b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.h
index 53a5f0f..7c63146 100644
--- a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.h
+++ b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPConnectionWorker.h
@@ -62,6 +62,8 @@ private:
IMAPMailbox* _MailboxFor(IMAPFolder& folder);
IMAPFolder* _Selected() const {
return fSelectedBox; }
void _SyncCommandDone();
+ uint32 _MessagesExist() const
+ {
return fMessagesExist; }
status_t _Connect();
void _Disconnect();
@@ -85,6 +87,7 @@ private:
IMAPFolder* fIdleBox;
IMAPFolder* fSelectedBox;
MailboxMap fMailboxes;
+ uint32 fMessagesExist;
BLocker fLocker;
thread_id fThread;
diff --git a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.cpp
b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.cpp
index 2c07be5..b32ea8c 100644
--- a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.cpp
+++ b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.cpp
@@ -25,10 +25,37 @@ IMAPMailbox::~IMAPMailbox()
void
-IMAPMailbox::AddMessageEntry(uint32 uid, uint32 flags, uint32 size)
+IMAPMailbox::AddMessageEntry(uint32 index, uint32 uid, uint32 flags,
+ uint32 size)
{
fMessageEntries.insert(
std::make_pair(uid, MessageFlagsAndSize(flags, size)));
+
+ fUIDs.reserve(index + 1);
+ fUIDs[index] = uid;
+}
+
+
+void
+IMAPMailbox::RemoveMessageEntry(uint32 index)
+{
+ if (index >= fUIDs.size())
+ return;
+
+ uint32 uid = fUIDs[index];
+ fMessageEntries.erase(uid);
+
+ fUIDs.erase(fUIDs.begin() + index);
+}
+
+
+uint32
+IMAPMailbox::UIDForIndex(uint32 index) const
+{
+ if (index >= fUIDs.size())
+ return 0;
+
+ return fUIDs[index];
}
diff --git a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.h
b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.h
index 352b2de..d1fa8ed 100644
--- a/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.h
+++ b/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPMailbox.h
@@ -6,6 +6,7 @@
#define IMAP_MAILBOX_H
+#include "Commands.h"
#include "IMAPFolder.h"
@@ -22,10 +23,14 @@ public:
const BString& MailboxName() const { return
fMailboxName; }
- void AddMessageEntry(uint32
uid, uint32 flags,
- uint32
size);
+ void AddMessageEntry(uint32
index, uint32 uid,
+ uint32
flags, uint32 size);
+ void
RemoveMessageEntry(uint32 index);
+
+ uint32 UIDForIndex(uint32
index) const;
uint32 MessageFlags(uint32
uid) const;
uint32 MessageSize(uint32 uid)
const;
+ uint32 CountMessages() const {
return fUIDs.size(); }
// FolderListener interface
virtual uint32 MessageAdded(const
MessageToken& fromToken,
@@ -53,6 +58,7 @@ protected:
IMAP::Protocol& fProtocol;
BString fMailboxName;
MessageEntryMap fMessageEntries;
+ IMAP::MessageUIDList fUIDs;
};
diff --git
a/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.cpp
b/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.cpp
index 149e921..9c5633c 100644
--- a/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.cpp
+++ b/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.cpp
@@ -24,6 +24,10 @@
using namespace BPrivate;
+/*! Maximum size of commands to the server (soft limit) */
+const size_t kMaxCommandLength = 2048;
+
+
static void
PutFlag(BString& string, const char* flag)
{
@@ -293,7 +297,11 @@ BString
FetchMessageEntriesCommand::CommandString()
{
BString command = fUIDs ? "UID FETCH " : "FETCH ";
- command << fFrom << ":" << fTo << " (FLAGS RFC822.SIZE";
+ command << fFrom;
+ if (fFrom != fTo)
+ command << ":" << fTo;
+
+ command << " (FLAGS RFC822.SIZE";
if (!fUIDs)
command << " UID";
command << ")";
@@ -336,10 +344,47 @@ FetchMessageEntriesCommand::HandleUntagged(Response&
response)
FetchCommand::FetchCommand(uint32 from, uint32 to, uint32 flags)
:
- fFrom(from),
- fTo(to),
fFlags(flags)
{
+ fSequence << from;
+ if (from != to)
+ fSequence << ":" << to;
+}
+
+
+/*! Builds the sequence from the passed in UID list, and takes \a max
entries
+ at maximum. If the sequence gets too large, it might fetch less entries
+ than specified. The fetched UIDs will be removed from the \uids list.
+*/
+FetchCommand::FetchCommand(MessageUIDList& uids, size_t max, uint32 flags)
+ :
+ fFlags(flags)
+{
+ // Build sequence string
+ max = std::min(max, uids.size());
+
+ size_t index = 0;
+ while (index < max && (size_t)fSequence.Length() < kMaxCommandLength) {
+ // Get start of range
+ uint32 first = uids[index++];
+ uint32 last = first;
+ if (!fSequence.IsEmpty())
+ fSequence += ",";
+ fSequence << first;
+
+ for (; index < max; index++) {
+ uint32 uid = uids[index];
+ if (uid != last + 1)
+ break;
+
+ last = uid;
+ }
+
+ if (last != first)
+ fSequence << ":" << last;
+ }
+
+ uids.erase(uids.begin(), uids.begin() + index);
}
@@ -354,9 +399,7 @@ BString
FetchCommand::CommandString()
{
BString command = "UID FETCH ";
- command << fFrom;
- if (fFrom != fTo)
- command << ":" << fTo;
+ command += fSequence;
command += " (UID ";
if ((fFlags & kFetchFlags) != 0)
@@ -525,10 +568,10 @@ ExistsHandler::HandleUntagged(Response& response)
if (!response.EqualsAt(1, "EXISTS") || !response.IsNumberAt(0))
return false;
- int32 index = response.NumberAt(0);
+ uint32 count = response.NumberAt(0);
if (fListener != NULL)
- fListener->MessageExistsReceived(index);
+ fListener->MessageExistsReceived(count);
return true;
}
@@ -570,7 +613,7 @@ ExpungeHandler::HandleUntagged(Response& response)
if (!response.EqualsAt(1, "EXPUNGE") || !response.IsNumberAt(0))
return false;
- int32 index = response.NumberAt(0);
+ uint32 index = response.NumberAt(0);
if (fListener != NULL)
fListener->MessageExpungeReceived(index);
diff --git a/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.h
b/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.h
index 8d4c042..1f9c31f 100644
--- a/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.h
+++ b/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/Commands.h
@@ -33,6 +33,8 @@ struct MessageEntry {
};
typedef std::vector<MessageEntry> MessageEntryList;
+typedef std::vector<uint32> MessageUIDList;
+
enum MessageFlags {
kSeen = 0x01,
kAnswered = 0x02,
@@ -161,6 +163,8 @@ class FetchCommand : public Command, public Handler,
public:
FetchCommand(uint32 from, uint32 to,
uint32
fetchFlags);
+
FetchCommand(MessageUIDList& uids,
+ size_t
max, uint32 fetchFlags);
void
SetListener(FetchListener* listener);
FetchListener* Listener() const { return
fListener; }
@@ -172,8 +176,7 @@ public:
size_t&
length);
private:
- uint32 fFrom;
- uint32 fTo;
+ BString fSequence;
uint32 fFlags;
FetchListener* fListener;
};
@@ -213,7 +216,7 @@ private:
class ExistsListener {
public:
- virtual void MessageExistsReceived(uint32
index) = 0;
+ virtual void MessageExistsReceived(uint32
count) = 0;
};
Other related posts:
- » [haiku-commits] BRANCH axeld-github.imap [af02a4f] in src/add-ons/mail_daemon/inbound_protocols/imap: . imap_lib - axeld-github . imap
