Безопасно ли использование карты кэша в многопоточной среде?

Question

Я реализую LRU-кэш для фотографий пользователей, используя Commons Collections LRUMap (который в основном представляет собой LinkedHashMap с небольшими изменениями). Метод findPhoto может вызываться несколько сотен раз в течение нескольких секунд.

public class CacheHandler {
    private static final int MAX_ENTRIES = 1000;
    private static Map<Long, Photo> photoCache = Collections.synchronizedMap(new LRUMap(MAX_ENTRIES));

    public static Map<Long, Photo> getPhotoCache() {
        return photoCache;
    }
}

Использование:

public Photo findPhoto(Long userId){
    User user = userDAO.find(userId);
    if (user != null) {
        Map<Long, Photo> cache = CacheHandler.getPhotoCache();

        Photo photo = cache.get(userId);
        if(photo == null){
            if (user.isFromAD()) {
                try {
                    photo = LDAPService.getInstance().getPhoto(user.getLogin());
                } catch (LDAPSearchException e) {
                    throw new EJBException(e);
                }
            } else {
                log.debug("Fetching photo from DB for external user: " + user.getLogin());
                UserFile file = userDAO.findUserFile(user.getPhotoId());
                if (file != null) {
                    photo = new Photo(file.getFilename(), "image/png", file.getFileData());
                }
            }
            cache.put(userId, photo);
        }else{
            log.debug("Fetching photo from cache, user: " + user.getLogin());
        }
        return photo;

    }else{
        return null;
    }
}

Как видите, я не использую блоки синхронизации. Я предполагаю, что наихудший сценарий здесь - это состояние гонки, при котором два потока запускают cache.put (userId, photo) для одного и того же userId. Но данные будут одинаковыми для двух потоков, так что это не проблема.

Правильно ли мое рассуждение здесь? Если нет, есть ли способ использовать блок синхронизации без значительного снижения производительности? Наличие только 1 потока, обращающихся к карте за один раз, ощущается как излишнее.

Answer 1

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

Answer 2

Ассилиас прав, что то, что у тебя есть, будет работать нормально.

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

Это довольно легко координировать, используя некоторые из более простых классов параллелизма Java.

Во-первых, позвольте мне реорганизовать ваш пример, чтобы извлечь интересную информацию. Вот что вы написали:

public Photo findPhoto(User user) {
    Map<Long, Photo> cache = CacheHandler.getPhotoCache();

    Photo photo = cache.get(user.getId());
    if (photo == null) {
        photo = loadPhoto(user);
        cache.put(user.getId(), photo);
    }
    return photo;
}

Вот, loadPhoto это метод, который выполняет загрузку фотографии, что здесь не актуально. Я предполагаю, что проверка пользователя выполняется в другом методе, который вызывает этот. Кроме этого, это ваш код.

Вместо этого мы делаем следующее:

public Photo findPhoto(final User user) throws InterruptedException, ExecutionException {
    Map<Long, Future<Photo>> cache = CacheHandler.getPhotoCache();

    Future<Photo> photo;
    FutureTask<Photo> task;

    synchronized (cache) {
        photo = cache.get(user.getId());
        if (photo == null) {
            task = new FutureTask<Photo>(new Callable<Photo>() {
                @Override
                public Photo call() throws Exception {
                    return loadPhoto(user);
                }
            });
            photo = task;
            cache.put(user.getId(), photo);
        }
        else {
            task = null;
        }
    }

    if (task != null) task.run();

    return photo.get();
}

Обратите внимание, что вам нужно изменить тип CacheHandler.photoCache для размещения упаковки FutureTask s. И так как этот код делает явную блокировку, вы можете удалить synchronizedMap от него. Вы также можете использовать ConcurrentMap для кэша, который позволил бы использовать putIfAbsent более параллельная альтернатива последовательности блокировки / получения / проверки для нулевой / положенной / разблокированной последовательности.

Надеюсь, то, что здесь происходит, довольно очевидно. Базовая схема получения чего-либо из кэша, проверки, имеет ли то, что вы получили, было нулевым, и если да, то что-то, возвращаемое обратно, все еще там. Но вместо того, чтобы положить в Photo Вы положили в Future который по сути является заполнителем для Photo которых может не быть (или может быть) там прямо в этот момент, но которые станут доступны позже. get метод на Future получает вещь, за которую удерживается место, блокируя, пока не прибудет, если необходимо.

Этот код использует FutureTask как реализация Future; это занимает Callable способен производить Photo в качестве аргумента конструктора, и вызывает его, когда его run метод называется. Призыв к run охраняется с помощью теста, который по существу повторяет if (photo == null) тест из ранее, но за пределами synchronized блок (потому что, как вы поняли, вы действительно не хотите загружать фотографии, удерживая блокировку кеша).

Это образец, который я видел или нуждался несколько раз. Обидно, что где-то не встроено в стандартную библиотеку.