[java] снова memory leaks

myclassMap = null; бессмысленно, так как вы переприсвоите значение все равно через несколько строчек

newMyClassMap = null; лишнее если эта переменная локальна.

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

Как корректно очищать такие статические коллекции?

Это зависит. Если логика приложения требует только *одной* такой коллекции, то присвоение ей null не гарантирует утекших ссылок.

не гарантирует утекших ссылок.

не гарантирует отсутствия утекших ссылок.

У вас есть проблема или нет?

есть. Или была - сейчас снова смотрю на графики, поработает часов 6 - узнаю точно.

Если логика приложения требует только *одной* такой коллекции, то присвоение ей null не гарантирует утекших ссылок.

Да, требуется только 1 такая коллекция. Как бороться с утекшими ссылками?

Как бороться с утекшими ссылками?

Железобетон - глубокое копирование при отдаче. Адекват - пользоваться обьектами из мапы минимально количество времени и не сохранять перманентно, можно например только локально внутри метода где надо. Вышло из метода, ссылки нет

А уж о том чтобы перманентно ссылаться на саму мапу где то еще вообще лучше не говорить

Да, требуется только 1 такая коллекция.

Тогда .clear(). Но при этом надо следить за потокобезопасностью (хотя следить за ней надо всегда, особенно в жабке).

А можно подробнее про оба варианта?
А то я уже запутался. Забирать из мапы обычным .get()? Потом никаких особых действий с переменной делать не надо?
PS: скоро скриншот выложу с графиками

Да, все правильно. Пользуйтесь мапой в ваших алгоритмах без проблем. Просто если у вас экземпляр класса висит в памяти все время, то не надо в его поля сохранять ссылку на мапу непосредственно

Попробовал сделать так:

// инициализация newMyClassMap ...
myclassMap.clear();
myclassMap = newMyClassMap;

Забирать из мапы обычным .get()? Потом никаких особых действий с переменной делать не надо?

Можно клонировать еще перед отдачей (RTFM Cloneable).

ЧЯДНТ?

1. myclassMap уже null

2. Если уж замещать inplace, то тогда можно делать это до конца. То есть элементы коллекции пихать сразу в myclassMap, хотя это не принципиально.

Не совсем понял.
Вот есть у меня сервлет, который запущен длительное время (в идеале - всегда). Он - экземпляр myclassServlet. У него поле private static Map... - откуда он и берет нужные данные и обслуживает http-запросы.
Как подразумевается работа:
- запускается приложение (с сервлетом)
- инициализируются данные (берется из базы, создаются объекты и пишутся в static HashMap)
- сервлет обслуживает запросы, берет данные из мапы, изменяет состояния объектов в ней и тд.
- иногда происходит добавление нового объекта (редко и мало), при этом происходит обновление мапы
- при перезапуске приложения - все очищается, а затем снова инициализируется.

1. myclassMap уже null

Т.е. по сути следующие действия эквивалентны? :
1. myclassMap = null;
2. myclassMap.clear();
3. myclassMap = newMyClassMap; (имеется в виду, что ссылки на старые объекты исчезнут и они будут собраны GC)

Ну если у вас один сервлет ссылается на мапу, то просто не парьтесь. И если вы редактируете мапу, то почему бы не использовать просто put, и не пересоздавать ее заново?

Т.е. вместо промежуточной локальной hashmap делать сразу put() в статическую? Да, может так и проще...но есть ли разница с точки зрения утечек, gc и тд?

Все 3 разные

1. Теряется ссылка на мапу. Если ссылок нигде на нее нет, то она удаляется. Если ссылок на обьекты внутри больше нет, то они тоже

2. Очищаются ссылки этой мапы на обьекты внутри. Если ссылок на обьекты внутри больше нет, то они удаляются

3. Смотри первый пункт, но при этом в переменной теперь будет не нулевая ссылка, а ссылка на новую пустую мапу

Т.е. по сути следующие действия эквивалентны?

При условии, что на эту коллекцию ссылается только один, текущий объект.

Естественно. Если вы при начальной инициализации вместо new HashMap() напишете Collections.synchronizedMap(new HashMap()), то можно безопасно работать во многих потоках с мапой с одним исключением - при итерации по элементам папы нужно итерацию завернуть в synchronized. А остальные операции будут атомарны.

И что тогда лучше делать?


Вот скрин:
http://gyazo.com/2c6d955020ce62bbd539a3c60080f365
оранжевое - OldGen
темно-зеленое - Survivor
голубое - Eden
светло-зеленое - all pools

«Пила» вроде не горизонтальная - утечка?

И что тогда лучше делать?

Не плодить велосипедов и юзать обычные put и get, предварительно обернув мапу в синхронизацию

Можете повызывать System.gc() почаще для теста и посмотреть как себя ведет

Скорее всего утекают jdbc'шные результаты. По этим графикам ничего конкретного сказать нельзя.

Для питона, например, я утечки ищу так:

* запускаю приложение
* определяю количество экземпляров для каждого класса
* пользуюсь приложением
* делаю замеры опять, и обращаю внимание на изменившиеся числа — это первые кандидаты в утечки.
* чтобы удостовериться, повторяю это несколько раз.

Все это упаковал в замечательную тулзу, которая может встраиваться в приложение и на определенном порту рисует вебинтерфейс для управления и вывода результатов.

Для жабки тоже самое делал ручками, но определенно должны существовать подобные утилиты.

Так, еще вопрос:
Тут возникло предположение, что это не утечка памяти в самом приложении, а проблема в настройках томката - там стоит время сессии 300 мин.
В результате накапливается до 90000 сессий (сейчас 40000).
И многие из них не активны, но TTL у них еще несколько часов.
Они жрут память? Может проблема быть в этом?

jdbc'шные результаты, подключения и тд - все закрываю.

Да, томкат такой. Вполне он может с ними долго сидеть. Но у вас какими-то ступенями все растет. Постарайтесь обратить внимание в какой момент это происходит

Зачем оборачивать, когда можно взять ConcurrentHashMap? ТС не говорил, что он сервер из музея взял.

Крупные срезы пил это релоад приложения?

Зачем оборачивать, когда можно взять ConcurrentHashMap?

Затем, что синхронизация нужно только при инициализации. Обезопасятся, млин, по самое не балуйся, а потом томми приходиться защищать честь.

Что-то я не понял, чем

Не плодить велосипедов и юзать обычные put и get, предварительно обернув мапу в синхронизацию

лучше ConcurrentHashMap, даже если это используется только при инициализации (что само по себе уже криво и не будет работать)?

И что значит

синхронизация нужно только при инициализации

? Т.е. ты предлагаешь делать синхронизованный доступ к хэшу только из места, где производится инициализация? А в других местах использовать доступ без синхронизации?

А зачем myclassMap - static? Это поле используется не только этим сервлетом, но и другими классами? Кто, кстати, обновляет его?

Т.е. ты предлагаешь делать синхронизованный доступ к хэшу только из места, где производится инициализация? А в других местах использовать доступ без синхронизации?

Чорт, я дурак, не подумал. Судя по условию, ТС нужна read-only коллекция, за исключением инициализации, и предполагаю в этот момент вообще посторонних объектов или нитей нет, так что нужность какой-либо синхронизации вообще под вопросом.

Если бы ты прочитал комментарий, на который я отвечал, то увидел бы, что vertexua там предложил использовать put(), вместо пересоздания коллекции. В этом случае я не считаю, что коллекция read-only.

что vertexua там предложил использовать put(), вместо пересоздания коллекции

вместо пересоздания коллекции

[offtop] И на фейхоа было городить gc, если оно всеравно течет? [/offtop]

Но у вас какими-то ступенями все растет. Постарайтесь обратить внимание в какой момент это происходит

Срезы - видимо, когда GC запускается

Зачем оборачивать, когда можно взять ConcurrentHashMap?

Смотрю snapshot'ы:
64% от общего объема памяти жрут java.util.concurrent.ConcurrentHashMap (хотя я их напрямую нигде не создаю).

А зачем myclassMap - static? Это поле используется не только этим сервлетом, но и другими классами? Кто, кстати, обновляет его?

Вот сам думаю, что может этот static не нужен. Тогда вопрос - вот запущен сервлет. Обрабатываются запросы. Создаются треды. Если поле будет не static, то никаких там проблем не возникнет с тредами?

ТС нужна read-only коллекция,

Не всегда. Я писал выше, что иногда, помимо инициализации, приходится добавлять 1-2 объекта в коллекцию.

Так течет не из-за GC, а из-за моих кривых рук.

Вот новый скрин графиков:
http://gyazo.com/c78803c112d7869f9f3db9265d3604ac

явная утечка.

Хотя и сессий в томкате уже 90000. Как точно определить, виноваты сессии или утечка это?

1. Посмотрел содержимое этих ConcurrentHashMap, которые reachable from GC via strong references - это действительно сессии. Т.е. видимо, утечки в сервлете нет - просто нужно уменьшить время сессии. Как это сделать? В web.xml томката стоит 30 мин, но в менеджере приложений все равно показывает 300 мин.

2. Так нужно ли делать эту коллекцию static? ТОЛЬКО в том случае, если доступ к ней происходит из других классов?

В общем, поправил время сессии на 20 мин - посмотрю как сейчас будет вести себя

Уж простите за простыню такую :)

Время сессии уменьшил, запустил GC - и сразу с 580МБ упало до 60 МБ. Посмотрим как через 20 мин будет.

Но вопрос еще остался - стоит ли все-таки делать мапу статической?

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

Что мне это должно сказать?

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

Ничего, ТС уже обмолвился, что коллекция не только на чтение. Посему, я был не прав.

Время сессии уменьшил

А они тебе вообще нужны, сессии?

Метод обновления hashmap - synchronized. Надо ли оборачивать в synchronized блоки везде, где put()/get() используются?

Используется только в сервлете. Т.е. спокойно можно делать не static? А треды?

Сессии нужны - там еще веб-морда есть для администрирования.

Что-то фигня какая-то: в web.xml поставил session timeout 20 min, а все равно куча сессий хранится и TTL у них несколько часов...Как убивать неактивные сессии?