0

我发现自己使用两种略有不同的方法从/中获取/创建项目ConcurrentHashMap，我想知道哪一种更好。

第一种方式：

public class Item {
  private boolean m_constructed;
  ...
  public void construct() {
    if (m_constructed) {
      return;
    }

    synchronized (this) {
      if (m_constructed) {
        return;
      }

      // Some heavy construction

      m_constructed = true;
    }
  }
}

ConcurrentHashMap<String, Item> m_items = new ConcurrentHashMap<String, Item>();

...
// The following code is executed concurrently by multiple threads:
public Item getOrCreateItem(String key) {
  Item newItem = new Item();                          // The constructor is empty
  Item item = m_items.putIfAbsent(key, newItem);
  if (item == null) {
    item = newItem;
  }
  item.construct();                                  // This is the real construction
  return item;
}

请不要评论在synchronize (this). 我知道this用作锁定对象的糟糕之处，但在这个特定的示例中我很好。

第二种方式：

public class Item {
  private boolean m_constructed;
  ...
  public void construct() {
    // Some heavy construction

    m_constructed = true;
  }

  public void waitForConstruction() throws InterruptedException {
    while (!m_constructed) {
      Thread.sleep(50);
    }
  }
}

ConcurrentHashMap<String, Item> m_items = new ConcurrentHashMap<String, Item>();

...
// The following code is executed concurrently by multiple threads:
public Item getOrCreateItem(String key) {
  Item newItem = new Item();                          // The constructor is empty
  Item item = m_items.putIfAbsent(key, newItem);
  if (item == null) {
    item.construct();                                 // This is the real construction
    item = newItem;
  }
  item.waitForConstruction();
  return item;
}

我想知道一种方法是否比另一种更优越。有任何想法吗？

编辑

关于上下文的几句话。该Item映射由多个线程同时填充，所有这些线程都执行getOrCreateItem。没有代码试图以任何其他方式访问地图。一旦人口结束，地图将永远不会被修改，并且对只读访问开放。因此，没有人可以在方法之外获得部分构造Item的实例getOrCreateItem。

编辑2

感谢所有的答案。我采用了第一种方法和建议的修复：

public class Item {
  private volatile boolean m_constructed;         // !!! using volatile
  ...
  public void construct() {
    if (m_constructed) {
      return;
    }

    synchronized (this) {
      if (m_constructed) {
        return;
      }

      // Some heavy construction

      m_constructed = true;
    }
  }
}

ConcurrentHashMap<String, Item> m_items = new ConcurrentHashMap<String, Item>();

...
// The following code is executed concurrently by multiple threads:
public Item getOrCreateItem(String key) {
  Item item = m_items.get(key);                         // !!! Checking the mainstream case first
  if (item == null) {
    Item newItem = new Item();                          // The constructor is empty
    item = m_items.putIfAbsent(key, newItem);
    if (item == null) {
      item = newItem;
    }
  }
  item.construct();                                  // This is the real construction
  return item;
}

当然，我假设没有代码使用除方法之外的任何其他方式访问项目映射getOrCreateItem。这在我的代码中是正确的。

4

2 回答 2

3

首先，请注意您的解决方案都没有正确同步。m_constructed 标志必须设置为 volatile 才能正常工作。否则你可能会遇到线程可见性问题，因为你对该成员的读取权限不受锁的保护。

无论如何， item 类与Future的概念太相似了。如果您将 Future 实现存储为映射值（例如FutureTask），您的问题就解决了。

于 2013-05-04T12:50:05.043 回答
1

我认为第一个解决方案还不错。m_constructed 变量必须是 volatile 才能正常工作，并且正如 John Vint 建议的那样，建议在执行 putIfAbsent() 之前调用 ConcurrentMap.get()。

我怀疑最重要的调用路径是稳态访问（线程访问已添加到地图并已构建的项目）。在这种情况下，使用第一个解决方案，您将执行 ConcurrentHashMap.get() 调用和 volatile 读取（在 m_constructed 上），这还不错。

第二种解决方案很糟糕，因为它涉及不必要的繁忙自旋循环。当根据 John Vint 的建议将其转换为使用 CountDownLatch 时，稳态性能将与上述类似： ConcurrentHashMap.get() 调用和 CountDownLatch.await() 应该类似于无竞争的 volatile 读取。然而，唯一的缺点是它给 Item 增加了更多的内存。

于 2013-05-04T18:29:04.290 回答