Синхронизация в кэше HashMap - PullRequest
7 голосов
/ 30 марта 2011

У меня есть веб-приложение, где люди просят ресурсы.Эти ресурсы кэшируются с использованием синхронизированной карты хеша для эффективности.Проблема здесь заключается в том, что два разных запроса приходят к одному и тому же некэшированному ресурсу одновременно: операция извлечения ресурсов занимает много памяти, поэтому я хочу не вызывать ее более одного раза для одного и того же ресурса.

Может кто-нибудь сказать мне, есть ли потенциальная проблема со следующим фрагментом?Заранее спасибо.

private Map<String, Resource> resources = Collections.synchronizedMap(new HashMap<String, Resource>());

public void request(String name) {

  Resource resource = resources.get(name);

  if (resource == null) {
    synchronized(this) {
      if (resources.get(name) == null) {
        resource = veryCostlyOperation(name); // This should only be invoked once per resource...
        resources.put(resource);
      } else {
        resource = resources.get(name);
      }
    }
  }

  ...

}

Ответы [ 4 ]

6 голосов
/ 30 марта 2011

Одной из возможных проблем является то, что вы создаете ненужную конкуренцию, выполняя veryCostlyOperation() внутри блока synchronized, так что многие потоки не могут получить свои (независимые) ресурсы одновременно. Это можно решить, используя Future<Resource> в качестве значений карты:

Map<String, Future<Resource>> map = new ConcurrentHashMap<String, Future<Resource>>();    
...
Future<Resource> r = map.get(name);
if (r == null) {
    FutureTask task = null;
    synchronized (lock) {
        r = map.get(name);
        if (r == null) {
            task = new FutureTask(new Callable<Resource>() {
                public Resource call() {
                    return veryCostlyOperation(name);
                }
            });
            r = task;
            map.put(name, r);
        }
    }
    if (task != null) task.run(); // Retrieve the resource
}

return r.get(); // Wait while other thread is retrieving the resource if necessary
2 голосов
/ 30 марта 2011

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

Я бы порекомендовал создать общий объект синхронизации и затем синхронизировать его с этим.

private final Object resourceCreationSynchObject = new Object();

тогда

synchronized(this.resourceCreationSynchObject) {
  ...
}

В противном случае, это именно то, что вы просите. Это гарантирует, что veryCostlyOperation не может быть вызван параллельно.

Также здорово подумать о повторном получении ресурса в блоке synchronized. Это необходимо, и первый внешний вызов гарантирует, что вы не синхронизируете, когда ресурс уже доступен. Но нет причин называть это в третий раз. Сначала в блоке synchronized установите resource снова на resources.get(name), а затем проверьте эту переменную на ноль. Это предотвратит повторный вызов get в предложении else.

1 голос
/ 30 марта 2011

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

  • Использование ConcurrentHashMap вместо синхронизированного HashMap позволит несколько вызовов метода get без блокировки.

  • Синхронизация по this вместо resources, вероятно, не нужна, но это зависит от остальной части вашего кода.

0 голосов
/ 31 марта 2011

Ваш код потенциально может вызывать veryCostlyOperation (name) несколько раз.Проблема в том, что после просмотра карты возникает несинхронизированный шаг:

public void request(String name) {
    Resource resource = resources.get(name);
    if (resource == null) {
        synchronized(this) {
            //...
        }
    }
    //...
}

Функция get () из карты синхронизируется картой, но проверка результата на нулевое значение ничем не защищена.Если несколько потоков вводят это, запрашивая одно и то же «имя», все они будут видеть нулевой результат от resources.get (), пока один из них фактически не завершит costlyOperation и не поместит ресурс в карту ресурсов.

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

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

private static class ResourceEntry {
     public Resource resource;
}

private Map<String, ResourceEntry> resources = new HashMap<String, ResourceEntry>();

public Resource request(String name) {
    ResourceEntry entry;
    synchronized (resources) {
        entry = resources.get(name);
        if (entry == null) {
            // if no entry exists, allocate one and add it to map
            entry = new ResourceEntry();
            resources.put(name, entry);
        }
    }
    // at this point we have a ResourceEntry, but it *may* be no loaded yet
    synchronized (entry) {
        Resource resource = entry.resource;
        if (resource == null) {
            // must create the resource
            resource = costlyOperation(name);
            entry.resource = resource;
        }
        return resource;
    }
}

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

...