このクラスはスレッドセーフですか?
class Counter {
private ConcurrentMap<String, AtomicLong> map =
new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
if (this.map.get(name) == null) {
this.map.putIfAbsent(name, new AtomicLong());
}
return this.map.get(name).incrementAndGet();
}
}
どう思いますか？
はい、マップを最終版にすれば可能です。if は必要ありませんが、必要に応じてパフォーマンス上の理由から保持できますが、ほとんどの場合、顕著な違いはありません。
public long add(String name) {
this.map.putIfAbsent(name, new AtomicLong());
return this.map.get(name).incrementAndGet();
}
編集
そのために、両方の実装をすばやくテストしました（チェックありとなし）。同じ文字列で 1,000 万回の呼び出しを行うと、次のようになります。
これは、私が言ったことを確認します。このメソッドを何百万回も呼び出すか、コードのパフォーマンスが重要な部分にない限り、違いはありません。
編集2
完全なテスト結果 -BetterCounterさらに良い結果が得られる を参照してください。現在、テストは非常に具体的 (競合なし + get は常に機能) であり、必ずしも使用法に対応しているわけではありません。
Counter: 482 ミリ秒
LazyCounter: 207 ミリ秒
MPCounter: 303 ミリ秒
BetterCounter: 135 ミリ秒
public class Test {
public static void main(String args[]) throws IOException {
Counter count = new Counter();
LazyCounter lazyCount = new LazyCounter();
MPCounter mpCount = new MPCounter();
BetterCounter betterCount = new BetterCounter();
//WARM UP
for (int i = 0; i < 10_000_000; i++) {
count.add("abc");
lazyCount.add("abc");
mpCount.add("abc");
betterCount.add("abc");
}
//TEST
long start = System.nanoTime();
for (int i = 0; i < 10_000_000; i++) {
count.add("abc");
}
long end = System.nanoTime();
System.out.println((end - start) / 1000000);
start = System.nanoTime();
for (int i = 0; i < 10_000_000; i++) {
lazyCount.add("abc");
}
end = System.nanoTime();
System.out.println((end - start) / 1000000);
start = System.nanoTime();
for (int i = 0; i < 10_000_000; i++) {
mpCount.add("abc");
}
end = System.nanoTime();
System.out.println((end - start) / 1000000);
start = System.nanoTime();
for (int i = 0; i < 10_000_000; i++) {
betterCount.add("abc");
}
end = System.nanoTime();
System.out.println((end - start) / 1000000);
}
static class Counter {
private final ConcurrentMap<String, AtomicLong> map =
new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
this.map.putIfAbsent(name, new AtomicLong());
return this.map.get(name).incrementAndGet();
}
}
static class LazyCounter {
private final ConcurrentMap<String, AtomicLong> map =
new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
if (this.map.get(name) == null) {
this.map.putIfAbsent(name, new AtomicLong());
}
return this.map.get(name).incrementAndGet();
}
}
static class BetterCounter {
private final ConcurrentMap<String, AtomicLong> map =
new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
AtomicLong counter = this.map.get(name);
if (counter != null)
return counter.incrementAndGet();
AtomicLong newCounter = new AtomicLong();
counter = this.map.putIfAbsent(name, newCounter);
return (counter == null ? newCounter.incrementAndGet() : counter.incrementAndGet());
}
}
static class MPCounter {
private final ConcurrentMap<String, AtomicLong> map =
new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
final AtomicLong newVal = new AtomicLong(),
prevVal = map.putIfAbsent(name, newVal);
return (prevVal != null ? prevVal : newVal).incrementAndGet();
}
}
}
編集
はい、マップを作成する場合final。そうしないと、すべてのスレッドが最初に呼び出したときに、マップ データ構造の最新バージョンが表示されることが保証されませんadd()。
複数のスレッドが の本体に到達できますif()。はputIfAbsent()、1 つだけAtomicLongがマップに配置されるようにします。
putIfAbsent()新しい値がマップにない限り、戻る方法はありません。
そのため、2 番目get()が実行されると、null値を取得することはなくAtomicLong、マップに追加できるのは 1 つのみであるため、すべてのスレッドが同じインスタンスを取得します。
[EDIT2]次の質問: これはどのくらい効率的ですか?
このコードは、不要な検索を回避するため高速です。
public long add(String name) {
AtomicLong counter = map.get( name );
if( null == counter ) {
map.putIfAbsent( name, new AtomicLong() );
counter = map.get( name ); // Have to get again!!!
}
return counter.incrementAndGet();
}
これが、キーが見つからないときに呼び出されるメソッドを持つGoogle のCacheBuilderを好む理由です。そうすれば、マップは 1 回だけ検索されるので、余分なインスタンスを作成する必要はありません。
誰も完全な解決策を持っていないようです:
public long add(String name) {
AtomicLong counter = this.map.get(name);
if (counter == null) {
AtomicLong newCounter = new AtomicLong();
counter = this.map.putIfAbsent(name, newCounter);
if(counter == null) {
counter = newCounter;
}
}
return counter.incrementAndGet();
}
これはどうですか：
class Counter {
private final ConcurrentMap<String, AtomicLong> map =
new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
this.map.putIfAbsent(name, new AtomicLong());
return this.map.get(name).incrementAndGet();
}
}
mapあることを保証する必要があります。final(詳細については、17.5 final フィールド セマンティクス (Java 言語仕様)を参照してください)if。何も監視していないことを願っています。編集: Java 言語仕様からの引用を追加しました:
このソリューション (addメソッドの本体のみを表示していることに注意してください。残りは同じままです!) への呼び出しを省略できますget。
final AtomicLong newVal = new AtomicLong(),
prevVal = map.putIfAbsent(name, newVal);
return (prevVal != null? prevVal : newVal).incrementAndGet();
おそらく、余分getなものは余分なものよりもはるかに高価ですnew AtomicLong()。
次のような方がいいと思います。
class Counter {
private ConcurrentMap<String, AtomicLong> map = new ConcurrentHashMap<String, AtomicLong>();
public long add(String name) {
AtomicLong counter = this.map.get(name);
if (counter == null) {
AtomicLong newCounter = new AtomicLong();
counter = this.map.putIfAbsent(name, newCounter);
if (counter == null) {
// The new counter was added - use it
counter = newCounter;
}
}
return counter.incrementAndGet();
}
}
そうしないと、複数のスレッドが同時に追加され、気付かないことがあります (putIfAbsent によって返される値を無視するため)。
マップを再作成することはないと思います。