Как оптимизировать метод?

Question

[Sushkov]

При помощи dotMemory выяснил что после вызова метода MailboxCheck(), начинается грузиться память
, хотя и вызываю сборщик мусора, он не помогает. Как быть?

void MailboxCheck(ImapClient client, String mailBox, String username)
        {
            client.DefaultMailbox = mailBox;
            IEnumerable<uint> uids = client.Search(SearchCondition.All());
            IEnumerable<MailMessage> messages = client.GetMessages(uids);

            foreach (MailMessage message in messages)
            {
                var attachments = message.Attachments;
                string path = folder + username;
                if (attachments.Count > 0)
                {
                    if (!Directory.Exists(path))
                    {
                        DirectoryInfo di = Directory.CreateDirectory(path);// Try to create the directory.
                    }
                }

                if (checktextBox.Checked && wordFind(message.Subject, message.Body, message.From.ToString(), message.To)) { continue; }

                foreach (Attachment attachment in attachments)
                {
                    string destinationFile = path + "\\" + attachment.Name;
                    try
                    {
                    
                        byte[] allBytes = new byte[attachment.ContentStream.Length];
                        using (var filestream = new FileStream(destinationFile, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None))
                        using (BinaryWriter writer = new BinaryWriter(filestream))
                        {
                            writer.Write(allBytes);
                            writer.Close(); //Хотя в принципе закроется при Dispose
                        }
                    }
                    catch
                    {
                        continue;
                    }

                }
            }
            GC.Collect();
        }

Comments

[Станислав Макаров]

Как насчет закомментить весь foreach и оставить только первые три строчки (вместе с GetMessages)? ImapClient - что за либа?

[Sushkov]

Станислав Макаров: S22.Imap, там же получается каждое письмо проверяем поэтому foreach

[Станислав Макаров]

Sushkov я понимаю, что каждое письмо, но есть соображение, что либа кэширует список сообщений где-то у себя. Видите, там есть List? Я предположу, что этот List вы видите как IEnumerable, и он где-то внутри либы торчит.

[Станислав Макаров]

Sushkov Хотя, судя по коду https://github.com/smiley22/S22.Imap/blob/master/I... список не сохраняется внутри клиента, т.е. отдается только вам. Попробуйте вызвать GetMessages много раз. Если у вас памяти много, возможно сборщик мусора пока не спешит собирать тот список что висит в памяти. Вызовите функцию 30-40 раз, посмотрим, будут ли списки висеть или нет.

[Sushkov]

Станислав Макаров: съело много памяти

[Станислав Макаров]

Sushkov во столько же раз больше, сколько раз вы и вызывали функцию?

[Sushkov]

Станислав Макаров: около 40 раз вызвал, 700MB накопиось в памяти

[Станислав Макаров]

Sushkov пока нет идей. А сколько всего памяти на машине, физической и файла подкачки?

[Sushkov]

Станислав Макаров: Все проблема в IEnumerable в нем письма сохраняются, как его очистить?

[Станислав Макаров]

Sushkov в том то и дело, что "очистка" сводится к уничтожению последней ссылки на тот список, который реализует этот IEnumerable. Внутри ImapClient ссылка вроде как не хранится, поэтому непонятно где еще она сохраняется, что не дает GC удалить объект. Вы точно нигде не сохраняете этот список?

[Sushkov]

Станислав Макаров: Вт если не сложно: https://ghostbin.com/paste/gxjwf

Answer 1

[Дмитрий Ковальский]

В корне я конечно алгоритм не улучшу, но на пару мелочей внимание обращу.
1) string path = folder + username; - Вы раз за разом пересчитываете эту переменную. Я не нашел внутри цикла изменений переменных - соответственно вы просто так создаете строку, потом убиваете ссылку и объект продолжает висеть до лучших времен.
2) if (checktextBox.Checked && wordFind(message.Subject, message.Body, message.From.ToString(), message.To)) { continue; }
Вы сначала делаете какие-то телодвижения, а потом, вне зависимости от результатов этих телодвижений, выясняете а есть ли в них смысл. Т. е. это просто лишние телодвижения - if можно поднять в самый верх.
3) writer.Close(); //Хотя в принципе закроется при Dispose - ну и зачем тогда?
4)

try
{}
catch{continue;}

Т.е. вы не имеете ни малейшего представления о том какие ошибки возникают при работе. Большое количество исключений в цикле здорово тормозит производительность, а вы даже не знаете возникают ли они. Возможно что всю конструкцию можно убрать добавив пару проверок для избавления от исключений.

Если честно - я бы переделал весь процесс сохранения файлов. Добавить очередь на сохранение и ряд потоков разбирающих эту очередь. Тогда циклом вы набиваете очередь и закрываете метод, а потом это все сохранится. Хотя не думаю что это лучшее решение, но в принципе имеет право на жизнь.

Comments

[Sushkov]

А как не только убить ссылку но еще и объект?

[Дмитрий Ковальский]

Sushkov: создайте 1 объект вне цикла, а не создавайте раз за разом. Тоже самое с проверкой directoryinfo - путь не меняется, а проверяете вы раз за разом.

[Sushkov]

Дмитрий Ковальский: А IEnumerable messages как освободить?

[Дмитрий Ковальский]

Sushkov: в момент выхода из цикла - никак. можете вызвать у коллекции clear()

[Sushkov]

Дмитрий Ковальский: У нее нет метода clear()

[georgich]

Что касается String. Лучше использовать StringBuilder: https://msdn.microsoft.com/ru-ru/library/2839d5h5%...

[Дмитрий Ковальский]

georgich: Не усложняйте) Там 1 операция конкатенации, а не склеивание строк циклом.