Является ли это incrementAndGet потокобезопасным?Вроде бы вытащить объект из ах кеша

Question

Этот сервлет, похоже, получает объект из ehCache, от Элемента, который имеет этот объект: http://code.google.com/p/adwhirl/source/browse/src/obj/HitObject.java?repo=servers-mobile

Затем происходит приращение счетчика с атомным длинным:

http://code.google.com/p/adwhirl/source/browse/src/servlet/MetricsServlet.java?repo=servers-mobile#174

    //Atomically record the hit
    if(i_hitType == AdWhirlUtil.HITTYPE.IMPRESSION.ordinal()) {
        ho.impressions.incrementAndGet();
    }
    else {
        ho.clicks.incrementAndGet();
    }

Это не кажется мне поточно-ориентированным, так как из кеша могут извлекаться несколько потоков, и если оба значения увеличиваются одновременно, вы можете потерять счетчик кликов / показов.

Согласны ли вы, что это не потокобезопасно?

Answer 1

AtomicLong и AtomicInteger используют CAS для внутреннего использования - сравнивают и устанавливают (или сравнивают и меняют). Идея состоит в том, что вы сообщаете CAS две вещи: значение, которое вы ожидаете от long / int, и значение, которое вы хотите обновить. Если long / int имеет значение, которое, как вы говорите, должно иметь, CAS автоматически выполнит обновление и вернет true; в противном случае обновление не произойдет, и будет возвращено false. Многие современные чипы очень эффективно поддерживают CAS на уровне машинного кода; если JVM работает в среде, в которой нет CAS, она может использовать мьютексы (то, что Java называет синхронизацией) для реализации CAS. В любом случае, получив CAS, вы можете безопасно реализовать атомарный инкремент с помощью этой логики (в псевдокоде):

long incrementAndGet(atomicLong, byIncrement)
    do
        oldValue = atomicLong.get()            // 1
        newValue = oldValue + byIncrement
    while ! atomicLong.cas(oldValue, newValue) // 2
    return newValue

Если вошел другой поток и сделал свой собственный шаг между строками // 1 и // 2, CAS завершится ошибкой, и цикл будет повторен. В противном случае CAS будет успешным.

В таком подходе есть риск: если конкуренция низкая, CAS работает быстрее, чем синхронизированный блок, что не вызывает переключения контекста потока. Но если есть много споров, некоторым потокам придется проходить многократные итерации цикла за приращение, что, очевидно, равносильно потраченной работе. Вообще говоря, incrementAndGet будет быстрее при наиболее распространенных нагрузках.

Answer 2

Приращение является потокобезопасным, поскольку AtomicInteger и семейство гарантируют это. Но есть проблема со вставкой и извлечением из кэша, где могут быть созданы и вставлены два (или более) HitObject. Это может привести к потере некоторых попаданий при первом обращении к этому HitObject. Как указал @ denis.solonenko, в коде уже есть TODO, чтобы это исправить.

Однако я хотел бы отметить, что этот код страдает от параллелизма только при первом доступе к данному HitObject. Если в кэше есть HitObject (и нет больше потоков, создающих или вставляющих HitObject), тогда этот код становится полностью поточно-ориентированным. Так что это только очень ограниченная проблема параллелизма, и, вероятно, по этой причине они еще не исправили ее.