Question

java中ConcurrentHashMap是线程安全的，我这样写有什么问题吗？

private static ConcurrentMap<String, Integer> KeyTotal = new ConcurrentHashMap<String, Integer>();

public static int GetKeyBM(String word) {
try{
int oldValue;
if (KeyTotal.containsKey(word)){
oldValue = KeyTotal.get(word);
}

else{
oldValue=KeyTotal.size()+1;

while (true) {
if (KeyTotal.putIfAbsent(word, oldValue) == null) {
break;
}
}
}

return oldValue;
}
catch (Exception e) {
e.printStackTrace();

return -1;
}
}

说明：
比如对10000篇文章进行分词，对每个分出来的词通过调用GetKeyBM方法，如果存在，则取出对应的编码，如果不存在，则加入KeyTotal中，并且给予一个编码，编码是int型的，顺序编码，就是KeyTotal中的变量数加一。
如果一个线程来写，目前看没有任何问题，但我起了5个线程，每个线程处理2000篇文章，不出错，但结果就不正常，连续运行两遍，结果都不一样，所以怀疑是不是多线程运行的时候在获取关键字编码的时候出的问题，请大神帮着看看，ConcurrentHashMap这样用有问题没有？还有就是有没有更好的办法来实现这样的功能。

Answer 1

这样使用是有问题的。

ConcurrentMap能够保证每一次调用（例如一次putIfAbsent）都是原子操作，不受多线程影响，但并不保证多次调用之间也是原子操作。

以上实现的GetKeyBM方法中，ConcurrentMap的方法被调用了许多次，不同线程之间必然存在着竞争关系，导致最终结果不正确。

现在的目标是，将下面描述的这一系列操作作为原子操作：

“对每个分出来的词通过调用GetKeyBM方法，如果存在，则取出对应的编码，如果不存在，则加入KeyTotal中，并且给予一个编码，就是KeyTotal中的变量数加一”

最直观的方法就是整块同步：

synchronized (KeyTotal) {
  Integer value = KeyTotal.get(word);
  if (value == null) {
    value = KeyTotal.size() + 1;
    KeyTotal.put(word, value);
  }
}

这样，使用普通的map就可以了。

如果你使用的是Java 8的话，ConcurrentMap有一个类似的方法 computeIfAbsent 可以使用：

KeyTotal.computeIfAbsent(word, k -> KeyTotal.size() + 1);

这样才能确保一次原子操作。

computeIfAbsent方法的作用是，如果word键值不存在，则使用第二个参数来生成一个值放入map中，等价于以下代码，并且是原子操作：

 V computeIfAbsent(K key, Function<? super K,? extends V> mappingFunction):
 
 if (map.get(key) == null) {
     V newValue = mappingFunction.apply(key);
     if (newValue != null)
         return map.putIfAbsent(key, newValue);
 }

正好与你的目标是一致的。

追问

computeIfAbsent方法只有Java 8才有吗？我现在用的是jdk1.6,是不是就没有这个方法？
整块同步是不是就是把你给出的代码替换掉GetKeyBM里原先的内容就可以了？效率上是不是会慢很多？

追答

computeIfAbsent是Java 8新增的功能，之前的版本都没有。
synchronized 相比 ConcurrentMap ，效果确实会低一些。
不过这是最便捷的方法，如果想不使用synchronized而实现原子操作，会很复杂的，可以参考 putIfAbsent 的源代码。