MailKit
MailKit copied to clipboard
jstedfast
Memory allocations in Fetch method
I was inspecting the memory allocations for the thread that works with MailKit and surprisingly Task<ImapToken> takes huge chunk of memory more than strings and memory streams. Not sure if this is caused by the Task itself or ImapToken inside the task. Can this be improved? May be using ValueTask<T> or ImapToken struct can help?
I am using synchronous versions of the Mailkit interface.
What tool are you using to get this info? The VS Diagnostics tools aren't showing Task<ImapToken> at all for me for some reason.
Also, I think it's the tasks and not the tokens - but I could be wrong on that since ImapToken is a class so it'd be a "pointer" to the ImapToken instance in the Task<T>.
Which means there's a lot of Task<T>'s for ImapTokens floating around in your heap for some reason.
Is your code sync or async? Both code paths use the same methods that return Task<ImapToken>, so both will create those objects - I'm just curious if it makes a difference.
Another question is: is this a single ImapClient connection? Or multiple? (and roughly how many?)
I am using single connection and sync methods only.
To make it simple, I created a small console app to reproduce this
using MailKit;
using MailKit.Net.Imap;
using System;
using System.Threading.Tasks;
ImapClient client = new ImapClient();
client.Connect("---");
client.Authenticate("---", "---");
client.Inbox.Open(FolderAccess.ReadWrite);
var items = client.Inbox.Fetch(0, -1, MessageSummaryItems.UniqueId);
GC.Collect();
await Task.Delay(2000);
items array in my account contains 17 000 items. Build with .net 6.0 (net48 memory usage is even worse +15%)
This is the result
I am using JetBrains dotTrace - it has trial version - This is JetBrains performance measuring tool but it still shows memory allocations.
I tried JetBrains dotMemory which give more information about memory allocations and I see the same picture
I just did very quick find and replace of Task<ImapToken> with ValueTask<ImapToken>
This is the result from dotMemory
with Task<ImapToken>
with ValueTask<ImapToken>
Even with ValueTask<T> async methods have their overhead. I can see that some allocations in ValueTask is now reported in ImapStream+<ReadTokenAsync>. I know you wrote it with bool runAsync for better maintenance but I am not sure this huge memory overhead worth it. Even with ValueTask<T> there will be some unnecessary allocations for sync methods. I would rather have separate sync method implementations (at least for critical paths)
I've seen zero allocation ValueTask alternatives https://github.com/alanmcgovern/ReusableTasks https://github.com/mgravell/PooledAwait
Yea, I was worried you might have been using the sync API and the overhead was due to that :-\
I know you wrote it with bool runAsync for better maintenance but I am not sure this huge memory overhead worth it.
You might be right... and that was one of my big fears with doing things that way. I had just hoped I was wrong :(
Seems like you are looking at the number of objects created over the lifetime of the program as opposed to the number of objects that are live at any given point in time.
There are probably ways to cache the (common) ImapTokens, at least, and maybe the common Task<ImapToken>s which should reduce the number of those objects created.
I'm working on a bunch of other optimizations to reduce memory usage by reusing temporary buffers which may have a bigger impact.
That is correct. But the lifetime of the program is one call to Fetch method (or at least the significant part of it) which generates 130k Task<T> allocations for single method call. Allocations are not cheap and deallocations with GC are even more expensive. And at some point of time GC will reclaim this memory and that will impact app performance.
As an example from a real app - I am using 4 threads per mail account (one instance of ImapClient per thread) to synchronize the mail folders in parallel. With 2 accounts the app memory rise to 1.5gb. If I turn off Mailkit threads. App memory consumption is only 250mb.
I am already thinking how I can turn sync methods returning task to "pure" sync methods. My first thought was - source generator. It seems there are already examples of this https://xoofx.com/blog/2018/12/26/generate-async-from-sync-code-with-roslyn/ https://github.com/dotnet/roslyn/issues/12931 https://github.com/maca88/AsyncGenerator
It seems to be viable
Allocations are not cheap and deallocations with GC are even more expensive.
Yep, which is why I have not closed it.
I just wanted to make sure we were both on the same page (and I wasn't sure my understanding of what I was seeing in dotMemory was correct because I'm not familiar with their UI).
I've got a lot of local changes that reduce memory allocations a fair bit, altho not so much with regards to ImapToken and Task<ImapToken>.
With 2 accounts the app memory rise to 1.5gb.
Thanks for this info - that helps put this issue in perspective.
I'm not sure if you've noticed, but someone has been submitting a lot of PR's lately to MimeKit that reduce memory allocations in MimeKit as well.
Reducing memory allocations will be my main focus for a while to try to get this under better control.
I noticed last night while working on this that I was able to halve the number of byte[] allocations and what I'd also like to do is significantly reduce the number of string allocations as well (which you've probably noticed some commits along those lines, but it's not enough).
FWIW, here's some screenshots of progress I made yesterday (still uncommitted because I'm trying to work out some design ideas).
Before:
After:
Having byte[] and string allocations reduced will be great! I was thinking that probably ReadOnlySpan<T> will be huge for MailKit and MimeKit in the future since you can completely avoid making byte[] and string copies when parsing. I guess this is not yet an options since the code is still targeting older version where this is unsupported.
Still any optimization for byte[] and string won't affect the allocations of Task<T>. I believe the easiest and safer approach for mitigating Task<T> allocations is to convert private and protected methods to return ValueTask<T>. and keep Task<T> for the public interface
It seems that in the latest frameworks ValueTask<T> is allocation free
https://github.com/dotnet/coreclr/pull/26310
ValueTask<T> has some limitations
https://devblogs.microsoft.com/dotnet/understanding-the-whys-whats-and-whens-of-valuetask/
but I don't think MailKit code await Tasks concurrently or multiple times.
I am planning to research the possibility to create a source generator and use it for all async methods that has runAsync argument to generate a sync version of the methods. If we have any success with that I'll let you know.
I spent a few hours to convert Fetch method and its dependency methods to pure 'sync' methods. This is the result (there are still Task<T> but I they come from Connect and Authenticate methods
with sync methods returning Task<T> - Fetch method completed in 6.4 sec
with sync methods returning T - Fetch completed in 2.5 sec
Time time for which both test completed is not very accurate because it depends from the server response. However I ran multiple tests and pure 'sync' version is consistently faster.
Those are the average times:
Sync with Task<T> - 4.5sec
Sync with T - 3.5 sec. - Uses 60mb less that Task<T> version
I was thinking that probably ReadOnlySpan<T> will be huge for MailKit and MimeKit...
That's actually one of the main things being used to reduce allocations in MimeKit right now (and I've started using it in a few places in MailKit as well) when the framework supports it.
I'm also starting to phase out older frameworks. I'll likely be dropping support for anything older than 4.6.2 in late April/early May, for example.
Still any optimization for byte[] and string won't affect the allocations of Task<T>.
Correct.
I believe the easiest and safer approach for mitigating Task<T> allocations is to convert private and protected methods to return ValueTask<T>. and keep Task<T> for the public interface
Yea, that is my current thinking as well.
but I don't think MailKit code await Tasks concurrently or multiple times.
Correct :)
I am planning to research the possibility to create a source generator and use it for all async methods that has runAsync argument to generate a sync version of the methods. If we have any success with that I'll let you know.
The naming convention I've been using in MimeKit and MailKit is actually doAsync, but I've already got some of the API "ported" to have both sync and async from the ground up (and that's the order I'm working in, ImapStream -> ImapEngine -> further up). I'm also trying to refactor things a bit to share as much logic as possible while I go so that it's not a full duplication of code.
While using dotMemory, I noticed that SslStream is all async under the public sync APIs so I wonder if it's possible to achieve good results w/o resorting to sync and async versions of all the underlying private I/O API's.
FWIW, for my fairly small test IMAP account, I was getting about 1.4 K ImapTokens instantiated at a cost of 42.2 KB memory.
With https://github.com/jstedfast/MailKit/commit/bcc579134cd22dfe3146c44aaf09dabbae7f7b83 applied, it's now 726 ImapTokens instantiated at a cost of 22.7 KB memory.
If I can do that for the Task<ImapToken>s as well, that would be pretty significant.
I refactored ImapStream to have fully sync versions (i.e. no calling an Async function with doAsync = false).
based on my findings on a (admittedly) tiny mailbox of 96 messages, byte[] allocations dropped by 2/3rds, MemoryStream allocations dropped by 100% (which was the main source of byte[] allocs), ImapToken allocations dropped by 1/2, and all of the ImapStream async state machine allocations went away.
That should amount to a significant savings for you. If you have a chance, give the latest builds a try.
I've got more potential string allocation eliminations I can do and I'm willing to continue moving up the stack splitting up sync and async logic to try and reduce async state machine allocations. I can also start trying to use ValueTask (sadly only available in netstandard2.1+ and net5.0+, so I'll have to figure out a nice way of supporting net4x and netstandard2.0).
FWIW, I sub'd to the Linux Kernel Mailing List and the GCC mailing list which should be high-traffic enough that I'll build up a significant Inbox over the next few days. I sub'd last night and am already up to nearly 1000 messages.
Looks great! Here comparison - both ran for ~17k message summaries
Before:
After
ValueTask (sadly only available in netstandard2.1+ and net5.0+, so I'll have to figure out a nice way of supporting net4x and netstandard2.0).
You can use ValueTask<T> with .net4x by adding this package https://www.nuget.org/packages/System.Threading.Tasks.Extensions/
In my testing, I went from 34.5 MB a week ago to 28.5 MB total allocations now (and I'm up 7,000 messages from a week ago, too - currently at ~8000 messages).
I don't think I can go much farther with this unless I have 2 complete code paths - 1 for sync and 1 for async.
The FetchSummaryItemsAsync method is at 10.66 MB and ProcessUntaggedResponseAsync is at 5.33 MB
Okay, so here's what I think will be my plan:
I'll make a release soon with the current fixes and long-term I'll start moving toward having a fully-sync implementation as well.
I've been doing a bit of reading and unfortunately it seems like there's no real way to get around that.
I tried the following patch to try and get this a little more sync, but it made it worse:
diff --git a/MailKit/Net/Imap/ImapCommand.cs b/MailKit/Net/Imap/ImapCommand.cs
index a029ce08..e19dd3bb 100644
--- a/MailKit/Net/Imap/ImapCommand.cs
+++ b/MailKit/Net/Imap/ImapCommand.cs
@@ -885,7 +885,8 @@ namespace MailKit.Net.Imap {
}
} else if (token.Type == ImapTokenType.Asterisk) {
// we got an untagged response, let the engine handle this...
- await Engine.ProcessUntaggedResponseAsync (doAsync, CancellationToken).ConfigureAwait (false);
+ token = await Engine.ReadTokenAsync (doAsync, CancellationToken).ConfigureAwait (false);
+ await Engine.ProcessUntaggedResponseAsync (token, doAsync, CancellationToken).ConfigureAwait (false);
} else if (token.Type == ImapTokenType.Atom && (string) token.Value == Tag) {
// the next token should be "OK", "NO", or "BAD"
token = await Engine.ReadTokenAsync (doAsync, CancellationToken).ConfigureAwait (false);
diff --git a/MailKit/Net/Imap/ImapEngine.cs b/MailKit/Net/Imap/ImapEngine.cs
index e46b11c0..35e0cdf0 100644
--- a/MailKit/Net/Imap/ImapEngine.cs
+++ b/MailKit/Net/Imap/ImapEngine.cs
@@ -140,6 +140,9 @@ namespace MailKit.Net.Imap {
const string GreetingSyntaxErrorFormat = "Syntax error in IMAP server greeting. {0}";
const int BufferSize = 4096;
+ static readonly Task<ImapUntaggedResult> HandledTask = Task.FromResult (ImapUntaggedResult.Handled);
+ internal static readonly Task CompletedTask = Task.FromResult (0);
+
static int TagPrefixIndex;
internal readonly Dictionary<string, ImapFolder> FolderCache;
@@ -1049,12 +1052,34 @@ namespace MailKit.Net.Imap {
}
#endif
- async ValueTask SkipLineAsync (bool doAsync, CancellationToken cancellationToken)
+ async Task SkipLineAsync (CancellationToken cancellationToken)
{
ImapToken token;
do {
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ token = await Stream.ReadTokenAsync (cancellationToken).ConfigureAwait (false);
+
+ if (token.Type == ImapTokenType.Literal) {
+ var buf = ArrayPool<byte>.Shared.Rent (BufferSize);
+ int nread;
+
+ try {
+ do {
+ nread = await Stream.ReadAsync (buf, 0, BufferSize, cancellationToken).ConfigureAwait (false);
+ } while (nread > 0);
+ } finally {
+ ArrayPool<byte>.Shared.Return (buf);
+ }
+ }
+ } while (token.Type != ImapTokenType.Eoln);
+ }
+
+ void SkipLine (CancellationToken cancellationToken)
+ {
+ ImapToken token;
+
+ do {
+ token = Stream.ReadToken (cancellationToken);
if (token.Type == ImapTokenType.Literal) {
var buf = ArrayPool<byte>.Shared.Rent (BufferSize);
@@ -1062,10 +1087,7 @@ namespace MailKit.Net.Imap {
try {
do {
- if (doAsync)
- nread = await Stream.ReadAsync (buf, 0, BufferSize, cancellationToken).ConfigureAwait (false);
- else
- nread = Stream.Read (buf, 0, BufferSize, cancellationToken);
+ nread = Stream.Read (buf, 0, BufferSize, cancellationToken);
} while (nread > 0);
} finally {
ArrayPool<byte>.Shared.Return (buf);
@@ -1074,6 +1096,16 @@ namespace MailKit.Net.Imap {
} while (token.Type != ImapTokenType.Eoln);
}
+ Task SkipLineAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ if (doAsync)
+ return SkipLineAsync (cancellationToken);
+
+ SkipLine (cancellationToken);
+
+ return CompletedTask;
+ }
+
static bool TryParseUInt32 (string text, int startIndex, out uint value)
{
#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER
@@ -1864,9 +1896,9 @@ namespace MailKit.Net.Imap {
async ValueTask UpdateStatusAsync (bool doAsync, CancellationToken cancellationToken)
{
var token = await ReadTokenAsync (ImapStream.AtomSpecials, doAsync, cancellationToken).ConfigureAwait (false);
- string name, value;
uint count, uid;
ulong modseq;
+ string name;
switch (token.Type) {
case ImapTokenType.Literal:
@@ -2008,18 +2040,254 @@ namespace MailKit.Net.Imap {
return false;
}
+ async Task<ImapUntaggedResult> ProcessUntaggedByeResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ if (token.Type == ImapTokenType.OpenBracket) {
+ var code = await ParseResponseCodeAsync (false, doAsync, cancellationToken).ConfigureAwait (false);
+ current.RespCodes.Add (code);
+ } else {
+ var text = (await ReadLineAsync (doAsync, cancellationToken).ConfigureAwait (false)).TrimEnd ();
+ current.ResponseText = token.Value.ToString () + text;
+ }
+
+ current.Bye = true;
+
+ // Note: Yandex IMAP is broken and will continue sending untagged BYE responses until the client closes
+ // the connection. In order to avoid this scenario, consider this command complete as soon as we receive
+ // the very first untagged BYE response and do not hold out hoping for a tagged response following the
+ // untagged BYE.
+ //
+ // See https://github.com/jstedfast/MailKit/issues/938 for details.
+ if (QuirksMode == ImapQuirksMode.Yandex && !current.Logout)
+ current.Status = ImapCommandStatus.Complete;
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedCapabilityResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ await UpdateCapabilitiesAsync (ImapTokenType.Eoln, doAsync, cancellationToken).ConfigureAwait (false);
+
+ // read the eoln token
+ await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedEnabledResponseAsync (string atom, bool doAsync, CancellationToken cancellationToken)
+ {
+ do {
+ var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ if (token.Type == ImapTokenType.Eoln)
+ break;
+
+ AssertToken (token, ImapTokenType.Atom, GenericUntaggedResponseSyntaxErrorFormat, atom, token);
+
+ var feature = (string) token.Value;
+ if (feature.Equals ("UTF8=ACCEPT", StringComparison.OrdinalIgnoreCase))
+ UTF8Enabled = true;
+ else if (feature.Equals ("QRESYNC", StringComparison.OrdinalIgnoreCase))
+ QResyncEnabled = true;
+ } while (true);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedFlagsResultAsync (string atom, bool doAsync, CancellationToken cancellationToken)
+ {
+ var keywords = new HashSet<string> (StringComparer.Ordinal);
+ var flags = await ImapUtils.ParseFlagsListAsync (this, atom, keywords, doAsync, cancellationToken).ConfigureAwait (false);
+
+ var folder = current.Folder ?? Selected;
+ folder.UpdateAcceptedFlags (flags, keywords);
+
+ var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ AssertToken (token, ImapTokenType.Eoln, GenericUntaggedResponseSyntaxErrorFormat, atom, token);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedNamespaceResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ await UpdateNamespacesAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedStatusResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ await UpdateStatusAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedOkNoOrBadResponseAsync (ImapUntaggedResult result, bool doAsync, CancellationToken cancellationToken)
+ {
+ var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ if (token.Type == ImapTokenType.OpenBracket) {
+ var code = await ParseResponseCodeAsync (false, doAsync, cancellationToken).ConfigureAwait (false);
+ current.RespCodes.Add (code);
+ } else if (token.Type != ImapTokenType.Eoln) {
+ var text = (await ReadLineAsync (doAsync, cancellationToken).ConfigureAwait (false)).TrimEnd ();
+ current.ResponseText = token.Value.ToString () + text;
+ }
+
+ return result;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedNumberResponseAsync (uint number, CancellationToken cancellationToken)
+ {
+ bool doAsync = true;
+
+ // we probably have something like "* 1 EXISTS"
+ var token = await Stream.ReadTokenAsync (cancellationToken).ConfigureAwait (false);
+
+ AssertToken (token, ImapTokenType.Atom, "Syntax error in untagged response. {0}", token);
+
+ var folder = current.Folder ?? Selected;
+ var atom = (string) token.Value;
+
+ if (current.UntaggedHandlers.TryGetValue (atom, out var handler)) {
+ // the command registered an untagged handler for this atom...
+ await handler (this, current, (int) number - 1, doAsync).ConfigureAwait (false);
+ } else if (folder != null) {
+ if (atom.Equals ("EXISTS", StringComparison.OrdinalIgnoreCase)) {
+ folder.OnExists ((int) number);
+ } else if (atom.Equals ("EXPUNGE", StringComparison.OrdinalIgnoreCase)) {
+ if (number == 0)
+ throw UnexpectedToken ("Syntax error in untagged EXPUNGE response. Unexpected message index: 0");
+
+ folder.OnExpunge ((int) number - 1);
+ } else if (atom.Equals ("FETCH", StringComparison.OrdinalIgnoreCase)) {
+ // Apparently Courier-IMAP (2004) will reply with "* 0 FETCH ..." sometimes.
+ // See https://github.com/jstedfast/MailKit/issues/428 for details.
+ //if (number == 0)
+ // throw UnexpectedToken ("Syntax error in untagged FETCH response. Unexpected message index: 0");
+
+ await folder.OnFetchAsync (this, (int) number - 1, cancellationToken).ConfigureAwait (false);
+ } else if (atom.Equals ("RECENT", StringComparison.OrdinalIgnoreCase)) {
+ folder.OnRecent ((int) number);
+ } else {
+ //Debug.WriteLine ("Unhandled untagged response: * {0} {1}", number, atom);
+ }
+ } else {
+ //Debug.WriteLine ("Unhandled untagged response: * {0} {1}", number, atom);
+ }
+
+ await SkipLineAsync (cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ Task<ImapUntaggedResult> ProcessUntaggedNumberResponseAsync (uint number, bool doAsync, CancellationToken cancellationToken)
+ {
+ if (doAsync)
+ return ProcessUntaggedNumberResponseAsync (number, cancellationToken);
+
+ // we probably have something like "* 1 EXISTS"
+ var token = Stream.ReadToken (cancellationToken);
+
+ AssertToken (token, ImapTokenType.Atom, "Syntax error in untagged response. {0}", token);
+
+ var folder = current.Folder ?? Selected;
+ var atom = (string) token.Value;
+
+ if (current.UntaggedHandlers.TryGetValue (atom, out var handler)) {
+ // the command registered an untagged handler for this atom...
+ handler (this, current, (int) number - 1, doAsync);
+ } else if (folder != null) {
+ if (atom.Equals ("EXISTS", StringComparison.OrdinalIgnoreCase)) {
+ folder.OnExists ((int) number);
+ } else if (atom.Equals ("EXPUNGE", StringComparison.OrdinalIgnoreCase)) {
+ if (number == 0)
+ throw UnexpectedToken ("Syntax error in untagged EXPUNGE response. Unexpected message index: 0");
+
+ folder.OnExpunge ((int) number - 1);
+ } else if (atom.Equals ("FETCH", StringComparison.OrdinalIgnoreCase)) {
+ // Apparently Courier-IMAP (2004) will reply with "* 0 FETCH ..." sometimes.
+ // See https://github.com/jstedfast/MailKit/issues/428 for details.
+ //if (number == 0)
+ // throw UnexpectedToken ("Syntax error in untagged FETCH response. Unexpected message index: 0");
+
+ folder.OnFetch (this, (int) number - 1, cancellationToken);
+ } else if (atom.Equals ("RECENT", StringComparison.OrdinalIgnoreCase)) {
+ folder.OnRecent ((int) number);
+ } else {
+ //Debug.WriteLine ("Unhandled untagged response: * {0} {1}", number, atom);
+ }
+ } else {
+ //Debug.WriteLine ("Unhandled untagged response: * {0} {1}", number, atom);
+ }
+
+ SkipLine (cancellationToken);
+
+ return HandledTask;
+ }
+
+ async Task<ImapUntaggedResult> ProcessRegisteredUntaggedResponseAsync (ImapUntaggedHandler handler, bool doAsync, CancellationToken cancellationToken)
+ {
+ // the command registered an untagged handler for this atom...
+ await handler (this, current, -1, doAsync).ConfigureAwait (false);
+ await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedListResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ // unsolicited LIST response - probably due to NOTIFY MailboxName or MailboxSubscribe event
+ await ImapUtils.ParseFolderListAsync (this, null, false, true, doAsync, cancellationToken).ConfigureAwait (false);
+
+ var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ AssertToken (token, ImapTokenType.Eoln, "Syntax error in untagged LIST response. {0}", token);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedMetadataResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ // unsolicited METADATA response - probably due to NOTIFY MailboxMetadataChange or ServerMetadataChange
+ var metadata = new MetadataCollection ();
+ await ImapUtils.ParseMetadataAsync (this, metadata, doAsync, cancellationToken).ConfigureAwait (false);
+ ProcessMetadataChanges (metadata);
+
+ var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ AssertToken (token, ImapTokenType.Eoln, "Syntax error in untagged LIST response. {0}", token);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUntaggedVanishedResponseAsync (ImapFolder folder, bool doAsync, CancellationToken cancellationToken)
+ {
+ await folder.OnVanishedAsync (this, doAsync, cancellationToken).ConfigureAwait (false);
+ await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
+ async Task<ImapUntaggedResult> ProcessUnknownUntaggedResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ {
+ // don't know how to handle this... eat it?
+ await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
+
+ return ImapUntaggedResult.Handled;
+ }
+
/// <summary>
/// Processes an untagged response.
/// </summary>
/// <returns>The untagged response.</returns>
/// <param name="doAsync">Whether or not asynchronous IO methods should be used.</param>
/// <param name="cancellationToken">The cancellation token.</param>
- internal async Task<ImapUntaggedResult> ProcessUntaggedResponseAsync (bool doAsync, CancellationToken cancellationToken)
+ internal Task<ImapUntaggedResult> ProcessUntaggedResponseAsync (ImapToken token, bool doAsync, CancellationToken cancellationToken)
{
- var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ //var token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
var folder = current.Folder ?? Selected;
- var result = ImapUntaggedResult.Handled;
- ImapUntaggedHandler handler;
string atom;
// Note: work around broken IMAP servers such as home.pl which sends "* [COPYUID ...]" resp-codes
@@ -2030,139 +2298,48 @@ namespace MailKit.Net.Imap {
atom = "OK";
} else if (token.Type != ImapTokenType.Atom) {
// if we get anything else here, just ignore it?
- Stream.UngetToken (token);
- await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
- return result;
+ return ProcessUnknownUntaggedResponseAsync (doAsync, cancellationToken);
} else {
atom = (string) token.Value;
}
- if (atom.Equals ("BYE", StringComparison.OrdinalIgnoreCase)) {
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ if (atom.Equals ("BYE", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedByeResponseAsync (doAsync, cancellationToken);
- if (token.Type == ImapTokenType.OpenBracket) {
- var code = await ParseResponseCodeAsync (false, doAsync, cancellationToken).ConfigureAwait (false);
- current.RespCodes.Add (code);
- } else {
- var text = (await ReadLineAsync (doAsync, cancellationToken).ConfigureAwait (false)).TrimEnd ();
- current.ResponseText = token.Value.ToString () + text;
- }
+ if (atom.Equals ("CAPABILITY", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedCapabilityResponseAsync (doAsync, cancellationToken);
- current.Bye = true;
+ if (atom.Equals ("ENABLED", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedEnabledResponseAsync (atom, doAsync, cancellationToken);
- // Note: Yandex IMAP is broken and will continue sending untagged BYE responses until the client closes
- // the connection. In order to avoid this scenario, consider this command complete as soon as we receive
- // the very first untagged BYE response and do not hold out hoping for a tagged response following the
- // untagged BYE.
- //
- // See https://github.com/jstedfast/MailKit/issues/938 for details.
- if (QuirksMode == ImapQuirksMode.Yandex && !current.Logout)
- current.Status = ImapCommandStatus.Complete;
- } else if (atom.Equals ("CAPABILITY", StringComparison.OrdinalIgnoreCase)) {
- await UpdateCapabilitiesAsync (ImapTokenType.Eoln, doAsync, cancellationToken).ConfigureAwait (false);
-
- // read the eoln token
- await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
- } else if (atom.Equals ("ENABLED", StringComparison.OrdinalIgnoreCase)) {
- do {
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ if (atom.Equals ("FLAGS", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedFlagsResultAsync (atom, doAsync, cancellationToken);
- if (token.Type == ImapTokenType.Eoln)
- break;
-
- AssertToken (token, ImapTokenType.Atom, GenericUntaggedResponseSyntaxErrorFormat, atom, token);
-
- var feature = (string) token.Value;
- if (feature.Equals ("UTF8=ACCEPT", StringComparison.OrdinalIgnoreCase))
- UTF8Enabled = true;
- else if (feature.Equals ("QRESYNC", StringComparison.OrdinalIgnoreCase))
- QResyncEnabled = true;
- } while (true);
- } else if (atom.Equals ("FLAGS", StringComparison.OrdinalIgnoreCase)) {
- var keywords = new HashSet<string> (StringComparer.Ordinal);
- var flags = await ImapUtils.ParseFlagsListAsync (this, atom, keywords, doAsync, cancellationToken).ConfigureAwait (false);
- folder.UpdateAcceptedFlags (flags, keywords);
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ if (atom.Equals ("NAMESPACE", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedNamespaceResponseAsync (doAsync, cancellationToken);
- AssertToken (token, ImapTokenType.Eoln, GenericUntaggedResponseSyntaxErrorFormat, atom, token);
- } else if (atom.Equals ("NAMESPACE", StringComparison.OrdinalIgnoreCase)) {
- await UpdateNamespacesAsync (doAsync, cancellationToken).ConfigureAwait (false);
- } else if (atom.Equals ("STATUS", StringComparison.OrdinalIgnoreCase)) {
- await UpdateStatusAsync (doAsync, cancellationToken).ConfigureAwait (false);
- } else if (IsOkNoOrBad (atom, out result)) {
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ if (atom.Equals ("STATUS", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedStatusResponseAsync (doAsync, cancellationToken);
- if (token.Type == ImapTokenType.OpenBracket) {
- var code = await ParseResponseCodeAsync (false, doAsync, cancellationToken).ConfigureAwait (false);
- current.RespCodes.Add (code);
- } else if (token.Type != ImapTokenType.Eoln) {
- var text = (await ReadLineAsync (doAsync, cancellationToken).ConfigureAwait (false)).TrimEnd ();
- current.ResponseText = token.Value.ToString () + text;
- }
- } else {
- if (uint.TryParse (atom, NumberStyles.None, CultureInfo.InvariantCulture, out uint number)) {
- // we probably have something like "* 1 EXISTS"
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
+ if (IsOkNoOrBad (atom, out var result))
+ return ProcessUntaggedOkNoOrBadResponseAsync (result, doAsync, cancellationToken);
- AssertToken (token, ImapTokenType.Atom, "Syntax error in untagged response. {0}", token);
-
- atom = (string) token.Value;
-
- if (current.UntaggedHandlers.TryGetValue (atom, out handler)) {
- // the command registered an untagged handler for this atom...
- await handler (this, current, (int) number - 1, doAsync).ConfigureAwait (false);
- } else if (folder != null) {
- if (atom.Equals ("EXISTS", StringComparison.OrdinalIgnoreCase)) {
- folder.OnExists ((int) number);
- } else if (atom.Equals ("EXPUNGE", StringComparison.OrdinalIgnoreCase)) {
- if (number == 0)
- throw UnexpectedToken ("Syntax error in untagged EXPUNGE response. Unexpected message index: 0");
-
- folder.OnExpunge ((int) number - 1);
- } else if (atom.Equals ("FETCH", StringComparison.OrdinalIgnoreCase)) {
- // Apparently Courier-IMAP (2004) will reply with "* 0 FETCH ..." sometimes.
- // See https://github.com/jstedfast/MailKit/issues/428 for details.
- //if (number == 0)
- // throw UnexpectedToken ("Syntax error in untagged FETCH response. Unexpected message index: 0");
-
- await folder.OnFetchAsync (this, (int) number - 1, doAsync, cancellationToken).ConfigureAwait (false);
- } else if (atom.Equals ("RECENT", StringComparison.OrdinalIgnoreCase)) {
- folder.OnRecent ((int) number);
- } else {
- //Debug.WriteLine ("Unhandled untagged response: * {0} {1}", number, atom);
- }
- } else {
- //Debug.WriteLine ("Unhandled untagged response: * {0} {1}", number, atom);
- }
+ if (uint.TryParse (atom, NumberStyles.None, CultureInfo.InvariantCulture, out uint number))
+ return ProcessUntaggedNumberResponseAsync (number, doAsync, cancellationToken);
- await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
- } else if (current.UntaggedHandlers.TryGetValue (atom, out handler)) {
- // the command registered an untagged handler for this atom...
- await handler (this, current, -1, doAsync).ConfigureAwait (false);
- await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
- } else if (atom.Equals ("LIST", StringComparison.OrdinalIgnoreCase)) {
- // unsolicited LIST response - probably due to NOTIFY MailboxName or MailboxSubscribe event
- await ImapUtils.ParseFolderListAsync (this, null, false, true, doAsync, cancellationToken).ConfigureAwait (false);
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
- AssertToken (token, ImapTokenType.Eoln, "Syntax error in untagged LIST response. {0}", token);
- } else if (atom.Equals ("METADATA", StringComparison.OrdinalIgnoreCase)) {
- // unsolicited METADATA response - probably due to NOTIFY MailboxMetadataChange or ServerMetadataChange
- var metadata = new MetadataCollection ();
- await ImapUtils.ParseMetadataAsync (this, metadata, doAsync, cancellationToken).ConfigureAwait (false);
- ProcessMetadataChanges (metadata);
+ if (current.UntaggedHandlers.TryGetValue (atom, out var handler))
+ return ProcessRegisteredUntaggedResponseAsync (handler, doAsync, cancellationToken);
- token = await ReadTokenAsync (doAsync, cancellationToken).ConfigureAwait (false);
- AssertToken (token, ImapTokenType.Eoln, "Syntax error in untagged LIST response. {0}", token);
- } else if (atom.Equals ("VANISHED", StringComparison.OrdinalIgnoreCase) && folder != null) {
- await folder.OnVanishedAsync (this, doAsync, cancellationToken).ConfigureAwait (false);
- await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
- } else {
- // don't know how to handle this... eat it?
- await SkipLineAsync (doAsync, cancellationToken).ConfigureAwait (false);
- }
- }
+ if (atom.Equals ("LIST", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedListResponseAsync (doAsync, cancellationToken);
- return result;
+ if (atom.Equals ("METADATA", StringComparison.OrdinalIgnoreCase))
+ return ProcessUntaggedMetadataResponseAsync (doAsync, cancellationToken);
+
+ if (atom.Equals ("VANISHED", StringComparison.OrdinalIgnoreCase) && folder != null)
+ return ProcessUntaggedVanishedResponseAsync (folder, doAsync, cancellationToken);
+
+ return ProcessUnknownUntaggedResponseAsync (doAsync, cancellationToken);
}
/// <summary>
diff --git a/MailKit/Net/Imap/ImapFolder.cs b/MailKit/Net/Imap/ImapFolder.cs
index 4fb033a5..7af3b2a0 100644
--- a/MailKit/Net/Imap/ImapFolder.cs
+++ b/MailKit/Net/Imap/ImapFolder.cs
@@ -319,7 +319,12 @@ namespace MailKit.Net.Imap {
static Task QResyncFetchAsync (ImapEngine engine, ImapCommand ic, int index, bool doAsync)
{
- return ic.Folder.OnFetchAsync (engine, index, doAsync, ic.CancellationToken);
+ if (doAsync)
+ return ic.Folder.OnFetchAsync (engine, index, ic.CancellationToken);
+
+ ic.Folder.OnFetch (engine, index, ic.CancellationToken);
+
+ return ImapEngine.CompletedTask;
}
async Task<FolderAccess> OpenAsync (ImapCommand ic, FolderAccess access, bool doAsync, CancellationToken cancellationToken)
@@ -5437,13 +5442,10 @@ namespace MailKit.Net.Imap {
OnMessageExpunged (new MessageEventArgs (index));
}
- internal async Task OnFetchAsync (ImapEngine engine, int index, bool doAsync, CancellationToken cancellationToken)
+ void EmitFetchEvents (int index, MessageSummary message)
{
- var message = new MessageSummary (this, index);
UniqueId? uid = null;
- await FetchSummaryItemsAsync (engine, message, doAsync, cancellationToken).ConfigureAwait (false);
-
if ((message.Fields & MessageSummaryItems.UniqueId) != 0)
uid = message.UniqueId;
@@ -5482,6 +5484,24 @@ namespace MailKit.Net.Imap {
OnMessageSummaryFetched (message);
}
+ internal void OnFetch (ImapEngine engine, int index, CancellationToken cancellationToken)
+ {
+ var message = new MessageSummary (this, index);
+
+ FetchSummaryItemsAsync (engine, message, false, cancellationToken);
+
+ EmitFetchEvents (index, message);
+ }
+
+ internal async Task OnFetchAsync (ImapEngine engine, int index, CancellationToken cancellationToken)
+ {
+ var message = new MessageSummary (this, index);
+
+ await FetchSummaryItemsAsync (engine, message, true, cancellationToken).ConfigureAwait (false);
+
+ EmitFetchEvents (index, message);
+ }
+
internal void OnRecent (int count)
{
if (Recent == count)
(Actually, this iteration of the patch might not even build, but the idea was that I split up ProcessUntaggedResponseAsync() into smaller bits and made ImapCommand.StepAsync() read the next token and pass it in, this way ProcessUntaggedResponseAsync() didn't have to await anything.
I'm working on unwinding some of the other lower-level I/O methods to avoid async/await overhead.
Unfortunately, I lost my access to my JetBrains license because I foolishly updated my email address for my account (used to be my @xamarin.com address) not realizing that the license they gave me was for @xamarin.com. D'oh!
I need to poke them and see if they can fix that for me.
JetBrains fixed my license issue so now I can use dotMemory again :)
I'm curious as to how much difference my last few commits made (forgot to tag most of them with this issue so they aren't all showing above).
Another fix I have sitting locally is this:
@@ -632,17 +635,13 @@ namespace MailKit.Net.Imap
return tokens[0];
return string.Format ("({0})", string.Join (" ", tokens));
}
- static IList<IMessageSummary> AsReadOnly (ICollection<IMessageSummary> collection)
+ static IList<IMessageSummary> AsReadOnly (IList<IMessageSummary> list)
{
- var array = new IMessageSummary[collection.Count];
-
- collection.CopyTo (array, 0);
-
- return new ReadOnlyCollection<IMessageSummary> (array);
+ return new ReadOnlyCollection<IMessageSummary> (list);
}
class FetchPreviewTextContext : FetchStreamContextBase
{
static readonly PlainTextPreviewer textPreviewer = new PlainTextPreviewer ();
Essentially, the existing code duplicates the List<IMessageSummary> into a new IMessageSummary[] in order to make a read-only collection. The purpose of duplicating is/was to reduce wasted space that List<T> may have allocated, but performance-wise, I'm not sure it's worth it.
I may need to find a better solution.
I see this as difference between v3.2.0 and latest master
latest master
This method has biggest potential for memory savings if converted to synchronous
Ok, this morning's batch of commits reduced memory allocations significantly for me.
I think you'll love these 2 performance improvements as well:
- commit 8ecc1da687f3842ab6798e3554d86a2f9173ecc3
- commit c3df51475235d2720aee39e6fc8000850938e4c7
Looks very good!
v3.2.0
latest master
Is it possible to get rid of Task<ImapUntaggedResult>