Memory allocations in Fetch method #1335
Comments
|
Yea, that looks like an insane amount of memory used by |
|
What tool are you using to get this info? The VS Diagnostics tools aren't showing 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 Which means there's a lot of Is your code sync or async? Both code paths use the same methods that return |
|
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);
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 This is the result from dotMemory with Task with ValueTask 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 I've seen zero allocation ValueTask alternatives |
|
Yea, I was worried you might have been using the sync API and the overhead was due to that :-\
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) 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 It seems to be viable |
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
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 |
|
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 Still any optimization for
I am planning to research the possibility to create a source generator and use it for all async methods that has |
|
I spent a few hours to convert Fetch method and its dependency methods to pure 'sync' methods. This is the result (there are still
with sync methods returning with sync methods returning 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: |
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.
Correct.
Yea, that is my current thinking as well.
Correct :)
The naming convention I've been using in MimeKit and MailKit is actually |
|
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 With bcc5791 applied, it's now 726 If I can do that for the |
|
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 |
You can use |
|
Awesome, thanks for the pointer! |
…alueTask Part of an ongoing series of fixes for issue #1335
These get spammed pretty hard with large mailboxes, so it's probably worth caching them in order to relieve GC pressure. Part of an ongoing set of fixes for issue #1335
This reduces memory usage when the client isn't requesting Flags/Keywords. Part of a set of ongoing fixes for issue #1335
|
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 |
|
Thanks for your efforts on this. |
|
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 |
…ods to reduce async/await overhead Another partial fix for issue #1335
|
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. |
… overhead Another partial fix for issue #1335
…ync logic More fixes for issue #1335
|
It's pretty much all sync API all the way through the Fetch() API now |
|
There's still a lot of ImapFolder and ImapUtils methods that will need to be split for sync/async, but Fetch()/FetchAsync() were obviously the biggest offenders simply because of how much data gets processed. Ideas for improvements: There are a HUGE number of strings that get allocated, many of which might be duplicate atoms (and possibly even qstrings). Potential solutions that I can think of are:
|
|
Implemented the ImapTokenCache idea here: #1629 Unfortunately, it seems not to really help much since the vast majority of tokens are either QStrings (not cached) or numeric tokens (also not cached since it wouldn't make sense). I could look into caching qstring tokens as well, but it gets more complicated if they aren't us-ascii. |
|
Okay, disregard previous comment... The new PR now caches quoted-string tokens as well and I further reduced memory allocations by re-using lookup keys for Dictionary.TryGetValue() and re-using expired linked-list nodes/keys/items. @ekalchev I could probably use your input on this cache stuff. How much does it improve things in your use-case? |
|
@ekalchev This should also help: jstedfast/MimeKit@19b73ba I've got a local patch to make I've got other patches which fix Unfortunately, dotMemory goes crazy with those patches and taking snapshots causes my test app to explode in memory and then crash, so not sure wtf is happening and it makes me nervous about committing those changes. |
|
@ekalchev even more exciting developments here: jstedfast/MimeKit#951 |
|
The MimeKit Rfc2047 decoder memory performance improvements have now been released as part of MimeKit 4.2.0. I also released MailKit 4.2.0 with a bunch of the sync/async fixes, but did not include the ImapTokenCache... I want to work out the details a bit more on that. Notably, I need to find a way to get rid of the IsAscii() checks I'm doing, either by tracking the "is ascii" state in ImapStream.cs when it parses the qstring -or- by modifying the ImapTokenCache to be able to support non-ascii values. |
|
I'm doing something unorthodox and I'm attempting to fetch messages inside of an AWS Lambda function. I wrote two implementations, one with Golang and one with F# to compare. Initially the Golang version was much faster, but I was able to get them somewhat close by upping the F# memory to 2048MB. Anything above that I didn't see much gain. Despite this, the Golang version is still about 200ms faster. Investigating further, most of the time is spent in Connect(), about 300ms. Everything else is <100ms combined. I don't think the difference here is due to language speeds, they ran about the same until I started adding the imap fetching. Is it possible that some of the improvements made to Fetch could also be made to Connect? |
|
@matthewp I believe they have been already. The reason that FETCH was ever really even an issue, though, is because of the sheer amount of data that is received in a FETCH request. All of the other methods return much smaller amounts of data which means they don't have quite as much overhead. That said, where exactly is the slow section of the Connect() method? I also haven't merged the ImapTokenCache stuff yet - I've been meaning to finish up that PR and write unit tests for it, but have been too busy with life and other things. |
|
@jstedfast Thanks, I haven't checked inside of Connect() so I'm not sure. I actually switched to ConnectAsync but it is the same. It may be possible that they are no slow parts, I'll dig into it more and report back. The equivalent go method is this one so if there is a speed difference I'll look there and let you know. |
|
Ok, so that looks like it'd be the equivalent of: client.Connect (hostname, 993, SecureSocketOptions.SslOnConnect);Is that what your code is doing? |
|
No, mine was |
|
I tend to try and recommend the enum variation because it is more explicit. |
|
I'm seeing a 50ms-75ms improvement using |
|
Wow, that was unexpected. |
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
Part of an ongoing effort to fix issue #1335
The last of the fixes for issue #1335
|
I'm going to call this complete now. |
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.
The text was updated successfully, but these errors were encountered: