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.

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),
+		fUIDs(uids),
-		fTo(to),
 		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
+		printf("IMAP: fetch %" B_PRIuSIZE "u headers\n", fUIDs.size());
-		// UIDs might become really large
-		uint32 to = fTo;
-		if (to - fFrom >= kMaxFetchEntries)
-			to = fFrom + kMaxFetchEntries - 1;
 
-		printf("IMAP: fetch headers from %lu to %lu\n", fFrom, to);
+		IMAP::FetchCommand fetch(fUIDs, kMaxFetchEntries,
-		IMAP::FetchCommand fetch(fFrom, to,
 			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;
+	MessageUIDList			fUIDs;
-	uint32					fTo;
+	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,
+			status_t status = WorkerPrivate(worker).SelectMailbox(*fFolder);
-				fNextUID);
 			if (status != B_OK)
 				return status;
 
-			fState = FETCH_ENTRIES;
+			fLastIndex = WorkerPrivate(worker).MessagesExist();
-			fFirstUID = fLastUID = fFolder->LastUID() + 1;
+			fFirstIndex = fMailbox->CountMessages() + 1;
-			fMailboxEntries = 0;
+			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
+			uint32 to = fLastIndex;
-			// UIDs might become really large
+			uint32 from = fFirstIndex + kMaxFetchEntries < to
-			uint32 from = fLastUID;
+				? fLastIndex - kMaxFetchEntries : fFirstIndex;
-			uint32 to = fNextUID;
-			if (to - from >= kMaxFetchEntries)
-				to = from + kMaxFetchEntries - 1;
 
 			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::MessageEntryList entries;
-			IMAP::FetchMessageEntriesCommand fetch(entries, from, to, true);
+			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,
+				fMailbox->AddMessageEntry(from + i, entries[i].uid,
-					entries[i].size);
+					entries[i].flags, entries[i].size);
-				fTotalBytes += 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 (from == 1) {
-				if (fMailboxEntries > 0) {
+				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					fFirstIndex;
-	uint32					fLastUID;
+	uint32					fLastIndex;
-	uint32					fNextUID;
-	uint32					fMailboxEntries;
 	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
+	}
 }
 
 

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;

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];
 }
 
 

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,
+			void				AddMessageEntry(uint32 index, uint32 uid,
-									uint32 size);
+									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;
 };
 
 

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;
+	command += fSequence;
-	if (fFrom != fTo)
-		command << ":" << fTo;
 
 	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);

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;
+			BString				fSequence;
-			uint32				fTo;
 			uint32				fFlags;
 			FetchListener*		fListener;
 };
@@ -213,7 +216,7 @@ private:
 
 class ExistsListener {
 public:
-	virtual	void				MessageExistsReceived(uint32 index) = 0;
+	virtual	void				MessageExistsReceived(uint32 count) = 0;
 };