ugeneunipro / ugene

UGENE is free open-source cross-platform bioinformatics software
http://ugene.net
GNU General Public License v2.0
207 stars 60 forks source link

UGENE-8051. 8GB file consumes 26GB RAM #1653

Closed EvelinaBiserova closed 1 month ago

EvelinaBiserova commented 1 month ago

В UGENE есть своя сущность MemoryLocker, которая захватывает ресурс "Память". Логично, что при открытии файла с большим кол-вом последовательностей, надо оценить, сколько памяти потребуется, и захватить с помощью MemLocker-a. Поэтому

EvelinaBiserova commented 1 month ago

Можно сделать ещё более простое решение: если последовательностей (аннотаций) в файле больше миллиона, не добавлять таблицу аннотаций. Минус в том, что миллион и одна последовательность в принципе открываются у меня за разумное время, да и на двух миллионах всё не так плохо, только окно (GUI) ненадолго подвисает на открытии (проверено на релизе): последовательности прочитываются за минуту, аннотации экспортируются и открываются за 2-3 минуты, тратится примерно 3 Гб оперативы на всё. Какую тогда использовать константу-ограничение -- я не знаю. Возможно, это сильно будет зависеть от машины, поэтому фикс предложен именно такой (зависящий от памяти). Пользовательский файл 27м последовательностей на моей машине 32 Гб оперативы не открылся.

Также по поводу теста: минимально допустимое кол-во памяти как ресурса в UGENE допустимо установить в 200 Мб. Можно добавить тест: открыть файл ~200 тыс последовательностей->объекта аннотаций не будет

yalgaer commented 1 month ago

Что бы я предложила:

Проблема с памятью GObject у нас уже возникала и была попытка ее решить в SequenceObject - этот объект предлагает API в котором нет необходимости иметь всю последовательность в памяти. Код который к этому не готов может использовать метод getWholeSequence, но это может только legacy код. Новый я бы такой не заапрувила.

Теперь у нас возникла подобная проблема и с Annotation object. Его надо начать преобразовывать подобным образом - избегать хранения данных в памяти и загружать только то, что попросят.

В addAnnotationsForMergedU2Sequence мы создаем новый AnnotationTableObject и создаем в нем все аннотации сразу. Это можно сделать иначе: писать пачками (по 1000-1000) и при этом сохранять состояние объекта как dataLoaded = false.

Таким образом проблема должна уйти из этого места, за исключением того что мы еще ранее скопили const QStringList& contigNames, const QVector& mergedMapping, что тоже может привести к переполнению.

Предположим что таким образом мы можем создать annotation table object произвольного размера (записывая аннотации в базу потоком) - но возникнет проблема в другом месте, где мы их смотрим (вьюшка)

До тех пор пока мы не решим проблему там - будет переполнение памяти.

Я бы начала решать эту проблему по частям, как описано выше.

Шаг 1: Ограничила бы создание слишком больших объектов с аннотациями, например - не более 100k аннотаций. С этого момента UGENE перестает падать. В твоем случае либо вовсе бы не создавала объект либо выдала бы log-error что после 100к аннотации создаваться перестали.

Шаг 2: Начала бы исправлять каждое место в отдельности. Это десятки мест часть из них независимы. Например добавила бы потоковость в addAnnotationsForMergedU2Sequence и связанным с ним кодом, но поставила бы проверку на загрузку такого большого объекта: либо загружайте частями (как sequence object), либо получите ошибку в U2OpStatus если объект оказался слишком большим.

EvelinaBiserova commented 1 month ago

Да, я тоже так хотела сделать. Как ты верно подметила а) это огромная работа, б) дело не только в том, что аннотации полностью загружаются в память, а что они там остаются на всё время существования. У последовательности единовременно хранится в оперативе только кэш, остальное в бд. С аннотациями это не так и все вью завязаны на этом "не так", т. е. что всё находится в памяти. До релиза я не успею это сделать. Я правильно поняла, что твоё предложение как хот-фикс -- загружать аннотации если их не более 100'000? И этого изменения будет достаточно для закрытия крэша?

Я считаю, что надо хотя бы миллион, сто тыщ -- это мало.

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

yalgaer commented 1 month ago

Я правильно поняла, что твоё предложение как хот-фикс -- загружать аннотации если их не более 100'000? И этого изменения будет достаточно для закрытия крэша?

Да, у UGENE есть пределы использования и если он о них честно сообщает и не выдает ложный результат этого достаточно.

Я считаю, что надо хотя бы миллион, сто тыщ -- это мало.

Если будет работать с миллионом используя разумное количество памяти (меньше 1-3Gb) - то это нормально.

Также хочу обсудить: что считаешь, нам нужны эти большие изменения для таблицы аннотаций?

Если рассматривать UGENE как тул для визуализации и его текущие возможности отображения, то поддержка таблиц с миллионами аннотаций не нужна - придется переосмысливать и переделывать все вьюшки.

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

Получается, не нужно сегодня поддерживать такие объемы, лучше отполировать то что есть для файлов разумного размера.

EvelinaBiserova commented 1 month ago

Хороший ответ, спасибо. Моё мнение, что для UGENE как для визуализатора также нужно поддерживать корректную работу с большим кол-вом аннотаций/групп аннотаций/таблиц аннотаций (последние в больших кол-вах почти не встречаются). Другой вопрос, что это была бы хорошая, но вряд ли популярная фича: в среднем, в обычных файлах немного аннотаций. Возможно, стоит исследовать, как сделано в других инструментах.

1м последовательностей = 1727Мб оперативы на виндовсе