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

Мне приходится работать с множеством классов, которые используются для хранения данных в ConcurrentHashMap и обычно выглядят так:

import java.util.concurrent.ConcurrentHashMap;

import java.util.concurrent.locks.ReentrantLock;

public class Data {
    private ConcurrentHashMap<String, SomeObject> concurrentHashMap = new ConcurrentHashMap<String, SomeObject>();
    private ReentrantLock lock = new ReentrantLock();

    public void add(String string, SomeObject someObject) {
        try {
            lock.lock();
            concurrentHashMap.put(string, someObject);
        }
        finally {
            lock.unlock();
        }
    public void remove(String string) {
        try {
            lock.lock();
            concurrentHashMap.remove(string);
        }
        finally {
            lock.unlock();
        }
    public SomeObject get(String string) {
        try {
            lock.lock();
            return concurrentHashMap.get(string);
        }
        finally {
            lock.unlock();
        } 
    }

person florian    schedule 03.03.2014    source источник


Ответы (3)


блокировка вообще нужна?

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

    public SomeObject add(String string, PARAMETERS_TO_CONSTRUCT_SomeObject) {
        SomeObject result = concurrentHashMap.get(string);        
        if (result == null) {
            result = new SomeObject(PARAMETERS_TO_CONSTRUCT_SomeObject);
            SomeObject old = map.putIfAbsent(string, result);
            if (old != null) {
                result = old;
            }
        }
        return result;
    }

Это гарантирует, что у вас всегда есть только один экземпляр SomeObject, связанный с данным ключом, и предотвращает ненужное создание экземпляра / выделение памяти.

имеет ли смысл делать ConcurrentHashMap изменчивым?

Я думаю, что лучший способ безопасно опубликовать ConcurrentHashMap в этом случае - это определить его как окончательный:

    private final ConcurrentHashMap<String, SomeObject> concurrentHashMap = new ConcurrentHashMap<>();
person AnatolyG    schedule 03.03.2014

блокировка вообще нужна? Потому что моя интуиция говорит, что это не так.

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

Может оказаться, что использование ConcurrentHashMap вместо простого HashMap было излишним, но даже это остается открытым.

имеет ли смысл делать ConcurrentHashMap изменчивым?

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

person Marko Topolnik    schedule 03.03.2014

блокировка вообще нужна?

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

в некоторых из этих классов ConcurrentHashMap изменчив - имеет ли смысл делать ConcurrentHashMap изменчивым?

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

private volatile ConcurrentHashMap<K, V> map;

public int method1(K key, V value) {
  ConcurrentHashMap<K, V> localMap = map;
  localMap.putIfAbsent(key, value);
  return localMap.size();
}

public int method2(K key, V value) {
  map.putIfAbsent(key, value);
  return map.size();
}

public void mutatorMethod() {
  map = new ConcurrentHashMap<K,V>();
}

Два метода рекламируют одно и то же: поставить значение ключа, если он еще не существует, и вернуть размер карты. Интересен случай, когда method2 может возвращать 0 при одновременном вызове mutatorMethod, если присвоение новой карте произошло между вызовами putIfAbsent и size.

person Brett Okken    schedule 03.03.2014