Как сделать обновление BigDecimal в потоке ConcurrentHashMap безопасным - PullRequest
Новая
       40

Как сделать обновление BigDecimal в потоке ConcurrentHashMap безопасным

16 голосов
/ 20 декабря 2011

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

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

Если это небезопасно, пожалуйста, объясните, что я должен сделать, чтобы обеспечить безопасность потока.

Нужно ли synchronizeполучить / поставить или есть лучший способ? 

private ConcurrentHashMap<String, BigDecimal> sumByAccount;

public void addToSum(String account, BigDecimal amount){
    BigDecimal newSum = sumByAccount.get(account).add(amount);
    sumByAccount.put(account, newSum);
}

Большое спасибо!

Обновление:

Спасибо всем за ответ,Я уже понял, что приведенный выше код не является потокобезопасным .

Спасибо Vint за предложение AtomicReference в качестве альтернативы synchronize.Я использовал AtomicInteger для хранения целочисленных сумм и раньше, и мне было интересно, есть ли что-то подобное для BigDecimal.

Является ли окончательный вывод о за и против двух?

Ответы [ 4 ]

14 голосов
/ 20 декабря 2011

Вы можете использовать синхронизированный, как предложили другие, но если хотите минимально блокирующее решение, вы можете попробовать AtomicReference в качестве хранилища для BigDecimal 

ConcurrentHashMap<String,AtomicReference<BigDecimal>> map;

public void addToSum(String account, BigDecimal amount) {
    AtomicReference<BigDecimal> newSum = map.get(account);
    for (;;) {
       BigDecimal oldVal = newSum.get();
       if (newSum.compareAndSet(oldVal, oldVal.add(amount)))
            return;
    }
}

Изменить - я объясню это больше:

AtomicReference использует CAS , чтобы атомарно назначить одну ссылку. Цикл говорит это.

Если текущее поле, хранящееся в AtomicReference == oldVal [их расположение в памяти, а не их значение], замените значение поля, хранящегося в AtomicReference, на oldVal.add(amount). Теперь, каждый раз, когда после цикла for вы вызываете newSum.get (), он будет иметь объект BigDecimal, который был добавлен в.

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

При умеренном конфликте потоков это будет более быстрой реализацией, а при высоком конфликте лучше использовать synchronized

4 голосов
/ 20 декабря 2011

То есть не безопасно, поскольку потоки A и B могут одновременно вызывать sumByAccount.get(account) (более или менее), поэтому ни один из них не увидит результат add(amount) другого. То есть вещи могут происходить в такой последовательности:

  • thread A вызывает sumByAccount.get("accountX") и получает (например) 10.0.
  • поток B вызывает sumByAccount.get("accountX") и получает то же значение, что и поток A: 10.0.
  • поток A устанавливает newSum, скажем, 10,0 + 2,0 = 12,0.
  • поток B устанавливает newSum (скажем) 10,0 + 5,0 = 15,0.
  • Тема А вызывает sumByAccount.put("accountX", 12.0).
  • поток B вызывает sumByAccount.put("accountX", 15.0), перезаписывая то, что делал поток A.

Один из способов исправить это - добавить synchronized к вашему addToSum методу или обернуть его содержимое в synchronized(this) или synchronized(sumByAccount). Другой способ, поскольку приведенная выше последовательность событий происходит только в том случае, если два потока одновременно обновляют одну и ту же учетную запись, может заключаться во внешней синхронизации на основе какого-либо объекта Account. Не видя остальной логики вашей программы, я не могу быть уверен.

3 голосов
/ 20 декабря 2011

Ваше решение не является потокобезопасным.Причина в том, что сумма может быть пропущена, поскольку помещаемая операция отделена от получаемой операции (поэтому новое значение, которое вы вводите в карту, может пропустить добавляемую сумму одновременно).

Самый безопасный способ сделать то, что вы хотите, - это синхронизировать ваш метод.

2 голосов
/ 20 декабря 2011

Да, вам нужно синхронизировать, так как в противном случае вы можете иметь два потока, каждый из которых получает одно и то же значение (для одного и того же ключа), скажем, A и поток 1 добавляют B к нему, а поток 2 добавляет C к нему и сохраняет его обратно. Теперь результатом будет не A + B + C, а A + B или A + C.

Что вам нужно сделать, так это заблокировать что-то общее для дополнений. Синхронизация по get / put не поможет, если вы не выполните 

synchronize {
    get
    add
    put
}

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

...