Пишем кеш с определенным временем хранения объектов с использованием java.util.concurrent

[Dair_Targ]

Как-раз пару дней назад похожую задачу на собеседовании решал.

[bald2b]

Это всё хорошо, но зачем очередной велосипед при наличии класса Cache в Google Guava, который делает ровно всё то же самое?

[nullc0de]

Наверное для людей, которые хотят более подробно изучить тему кеширования и понимать, что происходит внутри.

[iximiuz]

Велосипед безусловно имеет право жить и работать, но только зачем очередной велосипед на Хабре??

[jvIlya]

1. именование у вас не единообразно — где-то «currentTimeMillis», где-то «default_timeout».

2. ну и оно у вас вообще компилируется?? В имени класса выпишете, через «с», а в конструкторе через «s»:
public class Caсhe<K, V> {
…
public Cashe(long default_timeout) throws Exception {

3. Странный у вас hashCode(). В чем смысл того, что вы прибавляете «43*7» к hashCode() ключа?
public int hashCode() {
int hash = 7;
hash = 43 * hash + (this.key != null? this.key.hashCode(): 0);
return hash;
}

[Odis]

1, 2. Прошу прощение, у меня были только исходники и планшет. Писал пост на планшете, немного опечатался, т.к. пришлось выкидывать много лишнего. Исправил.

3. hashCode() — генерировала IDE.

[smbd]

По сути весь hashCode() можно написать как return key.hashCode(); если всё же считать, что key != null (а именно так на мой взгляд и следует считать, и мб кидать из конструктора NPE или IllegalArgument).

[Odis]

Мне нужно было быстро доставать объекты по ключу с потокобезопасной коллекции. Сразу же на ум пришел ConcurrentMap.

[Odis]

Сначала не совсем понял. Замечание принято. Спасибо.

[apangin]

Свежедобавленный элемент может быть тут же удален вследствие race condition между добавляющим и чистящим потоками, плохо!

[smbd]

Дополню другими замечаниями.

public V get(K key)
{
return globalMap.get(new Key(key, default_timeout));
}

Зачем здесь параметр default_timeout передавать в конструктор? Создайте для Key конструктор с одним аргументом key и используйте его. Всё равно же сравнения не по таймауту делаются. И вообще как-то на каждый get по объекту создавать — не самая лучшая идея…

Дублирование кода в методах put, setAll и addAll.

Для методов

public void setAll(Map<K, V> map) ...
public void setAll(Map<K, V> map, long timeout) ...

не нужно писать один и тот же код в реализации. Просто из первого метода вызывайте второй — иначе зачем вам жуткий копипаст? Для put и addAll аналогично.

[Odis]

Спасибо. Исправил. Опять же проблемой было написания поста на планшете. Обязуюсь в следующий раз писать без ошибок.

[bald2b]

Вообще сама идея сделать кеш с указываемым временем жизни для каждого объекта кеша малоудачна.

Пример:
Делаем default_timeout 3600000 (1 час)
Добавляем put(«key», 60000); // 1 минута
Но даже через 10 минут скорее всего данные будут жить, так как проверка проводится 1 раз в 20 минут.
Выход: добавить параметр shrinkerWorkInterval (как в JCS) или сделать время жизни всех ключей одинаковым (как в Guava)

Кроме того не радуют вызов
return globalMap.get(new Key(key, default_timeout));
Создание нового объекта при каждом получении объекта из кеша? Выше уже посоветовали хранить данные в ConcurrentHashMap и отдельно данные о времени жизни в другой коллекции.

[skorn]

Как по мне — поток проверки совершенно лишний.
Можно просто переопределить get и в момент запроса просто сравнивать время в ключе с текущим. Даже удалять не надо — просто возвращаете null, если time-to-live уже прошел.

[cursed]

И не запрашиваемые данные будут висеть в кеше пока уборщица не выдернет кабель питания?

[skorn]

Ну, удаляйте. Это уже ваше дело как себя вести с этими объектами. Может у вас будет обновление кеша по тем же ключам. Тогда удалять смысла нет — только лишняя операция.
Суть моего коммента была в том, что демон немножко бесполезен)

[Odis]

На это каждый смотрит со своей колокольни. Если в кеш будет вставка во много потоков больших объектов и обращение к этим объектам будет идти раз в пятилетку, то очень скоро мы получим OutOfMemory. А с демоном, если обращений совсем не будет то через некоторое время CHM станет пустой.

[skorn]

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

[ashofthedream]

Так этож кэш. Тут можно и SoftReference заюзать, что бы oome не получть*

[Odis]

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

[ashofthedream]

Я считаю, что плох тот кэш, благодаря которому, можно OOME словить. Хотя, думаю, в любом случае, до OOME далеко. До него еще будут несколько сборок мусора и тормоза такие (если хип у нас достаточно большой), что польза от данного кэша быстро убежит в минус бесконечности.

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

[darkdimius]

Лучше вместо того чтобы поток ходил по всем элементам в фоне, сделать кучу по времени жизни, и засыпать до момента когда нужно будет изъять ближайший.
При вставке элемента посылать потоку-уборщику interrupt(чтобы узнать что в вершине кучи мог появится элемент который нужно удалить раньше).