回答編集履歴

3

コードの間違いを訂正

2017/10/10 07:59

投稿

KSwordOfHaste
KSwordOfHaste

スコア18394

test CHANGED
@@ -12,7 +12,7 @@
12
12
 
13
13
  class LruCache3<K, V>(private val capacity: Int,
14
14
 
15
- private val initializer: (K) -> V) : Cache<K, V> {
15
+ private val initializer: (K) -> V) {
16
16
 
17
17
  val map = object: LinkedHashMap<K, V>(capacity, 1.0F, true) {
18
18
 
@@ -28,7 +28,7 @@
28
28
 
29
29
 
30
30
 
31
- override operator fun get(key: K): V {
31
+ operator fun get(key: K): V {
32
32
 
33
33
  val stamp = lock.writeLock()
34
34
 
@@ -96,7 +96,7 @@
96
96
 
97
97
  リスト1
98
98
 
99
- `override operator fun get(key: K): V = map.getOrPut(key) { initializer(key) }`
99
+ `operator fun get(key: K): V = map.getOrPut(key) { initializer(key) }`
100
100
 
101
101
 
102
102
 
@@ -172,7 +172,7 @@
172
172
 
173
173
 
174
174
 
175
- `override operator fun get(key: K): V = map.computeIfAbsent(key, initializer)`
175
+ `operator fun get(key: K): V = map.computeIfAbsent(key, initializer)`
176
176
 
177
177
 
178
178
 

2

高速化について追記

2017/10/10 07:59

投稿

KSwordOfHaste
KSwordOfHaste

スコア18394

test CHANGED
@@ -1,3 +1,83 @@
1
+ ###若干高速かも知れないコード(追記3)
2
+
3
+ 前回まで指摘ばかりのコメントでしたが、高速化についてコメントしてみます。
4
+
5
+
6
+
7
+ ```kotlin
8
+
9
+ import java.util.*
10
+
11
+
12
+
13
+ class LruCache3<K, V>(private val capacity: Int,
14
+
15
+ private val initializer: (K) -> V) : Cache<K, V> {
16
+
17
+ val map = object: LinkedHashMap<K, V>(capacity, 1.0F, true) {
18
+
19
+ override fun removeEldestEntry(eldest: MutableMap.MutableEntry<K, V>?): Boolean {
20
+
21
+ return size > capacity
22
+
23
+ }
24
+
25
+ }
26
+
27
+ val lock = java.util.concurrent.locks.StampedLock()
28
+
29
+
30
+
31
+ override operator fun get(key: K): V {
32
+
33
+ val stamp = lock.writeLock()
34
+
35
+ try {
36
+
37
+ return map.getOrPut(key) { initializer(key) }
38
+
39
+ } finally {
40
+
41
+ lock.unlockWrite(stamp);
42
+
43
+ }
44
+
45
+ }
46
+
47
+
48
+
49
+ // toStringも上記と同様にwriteLockの中で実行。記載は省略します
50
+
51
+ }
52
+
53
+ ```
54
+
55
+ 上記は機能的に質問者さんの実装と同等ですが、同期にjavaのsynchronizedブロックではなく、StampedLock(compare and write命令を使うもの)で実装したものです。キャッシュサイズは少し増やして100、アクセスはほぼヒットとなるよう0~99の範囲とし、javaの並列ストリームパイプライン(以下)で実行し、20回の平均を見てみました。また比較のため単一スレッドでもやってみました。(なお自分のPCは2コアしかありません...)
56
+
57
+ `IntStream.range(0, 1000000).parallel().forEach { cache[it / 15] }`
58
+
59
+
60
+
61
+ |対象クラス|シリアル/パラレル|標本平均(ms)|標本標準偏差(ms)|
62
+
63
+ |:--|:--|--:|--:|
64
+
65
+ |オリジナル|シリアル|63|16|
66
+
67
+ |オリジナル|パラレル|166|17|
68
+
69
+ |StampedLock版|シリアル|62|11|
70
+
71
+ |StampedLock版|パラレル|82|16|
72
+
73
+
74
+
75
+ 単一スレッドの場合、競合がおきないため並列実行よりずっと高速に動作しますが、どちらの実装も差はでません。一方、並列処理ではsynchronizedブロックよりcompare & writeの方が早いという結果が伺えます。
76
+
77
+
78
+
79
+ ###元のコメント
80
+
1
81
  解答ではありませんが。
2
82
 
3
83
 

1

追記

2017/10/10 06:32

投稿

KSwordOfHaste
KSwordOfHaste

スコア18394

test CHANGED
@@ -75,3 +75,25 @@
75
75
  ...
76
76
 
77
77
  ```
78
+
79
+
80
+
81
+ ---
82
+
83
+ 追記2:すみません、上記の指摘には足りない点があった気がします
84
+
85
+
86
+
87
+ Collections.synchronizedMapメソッドはjava.util.Collectionsのメソッドだと思います。そうだとするとkotlinはmapが単なるMutableMapであると解釈しアトミックでないgetOrPut拡張関数が用いられます。これがjava.util.concurrent.ConcurrentMapであればアトミック版のgetOrPutが用いられるのですが、ConcurrentMapにはCollections.synchronizedMapのようにコレクションをラップしてくれるような適当なクラスが用意されておらず、LinkedHashMapの特徴を生かすことができません。
88
+
89
+
90
+
91
+ ということで、getOrPutをjava.util.concurrent.locks.ReentrantLockなどにより自前で同期するか、あるいはgetOrPutではなくjava側で同期が保証されるcomputeIfAbsentを使うなどの対処が必要と思います。後者の方が以下のようにシンプルに書けます。
92
+
93
+
94
+
95
+ `override operator fun get(key: K): V = map.computeIfAbsent(key, initializer)`
96
+
97
+
98
+
99
+ 自分がKotlinに暗いのとimport文が質問文に明記されていなかったため、本指摘があたりかどうか確信がもてませんが、一応コメントいたします。