7

I've got a web application where people ask for resources. This resources are cached using a synchronized hash map for efficiency. The problem here is when two different requests come for the same uncached resource at the same time: the operation retrieving the resources takes up a lot of memory, so I want to avoid calling it more than once for the same resource.

Can somebody please tell me if there is any potential problem with the following snippet? Thanks in advance.

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

4 回答 4

6

一个可能的问题是您通过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
于 2011-03-30T15:24:09.977 回答
2

我看到的唯一潜在问题是您同步到this. 如果同一类中的任何其他代码也同步到this，则这些块中只有一个会同时运行。也许没有其他东西可以做到这一点，这很好。不过，我总是担心下一个程序员会做什么。（或者三个月后我忘记了这段代码的我自己）

我建议创建一个通用同步对象，然后同步到那个对象。

私有最终对象 resourceCreationSynchObject = new Object();

然后

同步（this.resourceCreationSynchObject）{
  ...
}

否则，这正是您所要求的。它确保veryCostlyOperation不能并行调用。

synchronized此外，在块内第二次重新获取资源是一个很好的想法。这是必要的，并且在外部的第一次调用可确保您在资源已经可用时不进行同步。但是没有理由第三次调用它。synchronized块内的第一件事，resource再次设置为resources.get(name)然后检查该变量是否为空。这将防止您不得不在子句中get再次调用。else

于 2011-03-30T15:06:59.713 回答
1

您的代码看起来不错，只是您的同步超出了实际需要：

  • 使用 aConcurrentHashMap而不是 synchronizedHashMap将允许在不锁定的情况下多次调用 get 方法。

  • 同步 onthis而不是resources可能不是必需的，但这取决于您的代码的其余部分。

于 2011-03-30T15:07:38.333 回答
0

您的代码可能会多次调用veryCostlyOperation(name)。问题是查图后有一个不同步的步骤：

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

地图中的 get() 由地图同步，但检查结果是否为 null 不受任何保护。如果多个线程输入这个请求相同的“名称”，所有线程都会从 resources.get() 中看到空结果，直到真正完成 costlyOperation 并将资源放入资源映射中。

一种更简单且有效但扩展性较差的方法是使用法线贴图并使整个请求方法同步。除非它实际上在实践中出现问题，否则我会选择简单的方法。

为了获得更高的可伸缩性，您可以通过在 synchronized(this) 后再次检查地图来修复代码以捕获上述情况。它仍然不能提供最好的可伸缩性，因为 synchronized(this) 只允许一个线程执行 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 上进行同步以确保特定资源只构建一次。

于 2011-03-30T21:56:13.670 回答