Сохранять «очевидное» извлечение блокировки или использовать двойную проверку блокировки?

Question

Я сосу на формулировку вопросов. У меня есть следующий фрагмент кода (Java) (псевдо):

public SomeObject getObject(Identifier someIdentifier) {
    // getUniqueIdentifier retrieves a singleton instance of the identifier object,
    // to prevent two Identifiers that are equals() but not == (reference equals) in the system.
    Identifier singletonInstance = getUniqueIdentifier(someIdentifier);
    synchronized (singletonInstance) {
        SomeObject cached = cache.get(singletonInstance);
        if (cached != null) {
            return cached;
        } else {
            SomeObject newInstance = createSomeObject(singletonInstance);
            cache.put(singletonInstance, newInstance);
            return newInstance;
        }
    }
}

По сути, он делает идентификатор «уникальным» (ссылка равна, как в ==), проверяет кеш, а в случае его отсутствия вызывает дорогой метод (включая вызов внешнего ресурса, анализ и т. Д.) , помещает это в кеш и возвращает. Синхронизированный Identifier в этом случае избегает использования двух equals(), но не == Identifier объектов, используемых для вызова дорогостоящего метода, который бы одновременно получал один и тот же ресурс.

Выше работает. Мне просто интересно, и, возможно, микрооптимизация, будет ли перезапись, такая как следующая, которая использует более наивное извлечение кеша и двойную проверку блокировки, быть «безопасной» (безопасной, как в поточно-безопасной, без лишних условий гонки) и быть «более оптимальный '(как в сокращении ненужных блокировок и потоков, ожидающих блокировки)?

public SomeObject getObject(Identifier someIdentifier) {

    // just check the cache, reference equality is not relevant just yet.
    SomeObject cached = cache.get(someIdentifier);
    if (cached != null) {
        return cached;
    }        

    Identifier singletonInstance = getUniqueIdentifier(someIdentifier);
    synchronized (singletonInstance) {
        // re-check the cache here, in case of a context switch in between the 
        // cache check and the opening of the synchronized block.
        SomeObject cached = cache.get(singletonInstance);
        if (cached != null) {
            return cached;
        } else {
            SomeObject newInstance = createSomeObject(singletonInstance);
            cache.put(singletonInstance, newInstance);
            return newInstance;
        }
    }
}

Вы можете сказать «Просто протестируйте это» или «Просто сделайте микропроцессор», но тестирование многопоточных фрагментов кода не является моей сильной стороной, и я сомневаюсь, что смогу смоделировать реалистичные ситуации или точно фальшивые условия гонки. Кроме того, это заняло бы у меня полдня, в то время как написание ТАКОГО вопроса у меня заняло всего несколько минут:).

Answer 1

Если «кеш» - это карта (что, я подозреваю, так и есть), то эта проблема сильно отличается от простой проблемы двойной проверки блокировки.

Если кеш - это простая HashMap, то проблема заключается в том, чтона самом деле намного хуже;т. е. ваш предложенный «шаблон двойной проверки» ведет себя намного хуже, чем простая двойная проверка на основе ссылок.Фактически, это может привести к ConcurrentModificationExceptions, получению неверных значений или даже к бесконечному циклу.

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

public SomeObject getObject(Identifier someIdentifier) {
    // cache is a ConcurrentHashMap

    // just check the cache, reference equality is not relevant just yet.
    SomeObject cached = cache.get(someIdentifier);
    if (cached != null) {
        return cached;
    }        

    Identifier singletonInstance = getUniqueIdentifier(someIdentifier);
    SomeObject newInstance = createSomeObject(singletonInstance);
    SombObject old = cache.putIfAbsent(singletonInstance, newInstance);
    if (old != null) {
        newInstance = old;
    }
    return newInstance;
}

Answer 2

Вы заново изобретаете Google-Коллекции / MapMaker / ComputingMap от Guava:

ConcurrentMap<Identifier, SomeObject> cache = new MapMaker().makeComputingMap(new Function<Identifier, SomeObject>() {
  public SomeObject apply(Identifier from) {
    return createSomeObject(from);
  }
};

public SomeObject getObject(Identifier someIdentifier) {
  return cache.get(someIdentifier);
}

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

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

Answer 3

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

Кстати, вы можете написать

SomeObject cached = cache.get(singletonInstance);
if (cached == null) 
   cache.put(singletonInstance, cached = createSomeObject(singletonInstance));
return cached;