Odis Mar 19 2012 at 00:56

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

4 min

21K

Java*

From sandbox

+21

Comments 24

Dair_Targ Mar 19 2012 at 02:57

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

bald2b Mar 19 2012 at 07:23

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

nullc0de Mar 19 2012 at 09:48

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

iximiuz Mar 19 2012 at 16:30

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

jvIlya Mar 19 2012 at 09:54

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 Mar 19 2012 at 10:34

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

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

smbd Mar 19 2012 at 10:41

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

UFO just landed and posted this here

Odis Mar 19 2012 at 10:43

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

UFO just landed and posted this here

Odis Mar 19 2012 at 11:01

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

apangin Mar 19 2012 at 10:13

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

smbd Mar 19 2012 at 10:37

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

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 Mar 19 2012 at 10:51

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

bald2b Mar 19 2012 at 11:10

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

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

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

skorn Mar 19 2012 at 14:26

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

cursed Mar 19 2012 at 14:31

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

skorn Mar 19 2012 at 15:02

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

Odis Mar 19 2012 at 15:15

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

skorn Mar 19 2012 at 15:20

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

ashofthedream Mar 20 2012 at 01:45

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

Odis Mar 20 2012 at 10:31

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

ashofthedream Mar 20 2012 at 14:16

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

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

darkdimius Mar 23 2012 at 02:04

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

Show the best of all time