0

Google Guava Cache を使用して ConcurrentHashMaps のスレッドセーフなシングルトン キャッシュを作成しようとしています。これらの各マップにはリストが含まれます。リストは、リストに追加できるすべてのスレッドが実行された後、一度だけ読み取られます。私の実装 (具体的にはアイテムを更新する場所) がスレッドセーフかどうか、またはそれを改善する方法を考えています。同期ブロックを使用せずにこれを行うより良い方法はありますか?

public enum MyCache {
    INSTANCE;

    private static Cache<Integer, ConcurrentHashMap<String, List>> cache = 
        CacheBuilder.newBuilder()
            .maximumSize(1000)
            .build();

    private static AtomicInteger uniqueCount = new AtomicInteger(0);

    private final Object mutex = new Object();

        //Create a new unique ConcurrentHashMap
    public Integer newMapItem(){

        Integer key = uniqueCount.incrementAndGet();

        //We dont care if something exists
        cache.put(
            key,
            new ConcurrentHashMap<String, List>()
        );


        return key;
    }

    public void expireMapItem(int key){
        cache.invalidate(key);
    }

    public Integer add(int cacheKey, String mapListKey, int value){

        synchronized(mutex){
            ConcurrentMap<String, List> cachedMap = cache.getIfPresent(cacheKey);
            if (cachedMap == null){
                //We DONT want to create a new map automatically if it doesnt exist
                return null;
            }


            List mappedList = cachedMap.get(mapListKey);

            if(mappedList == null){



                List newMappedList = new List();
                mappedList = cachedMap.putIfAbsent(mapListKey, newMappedList);
                if(mappedList == null){
                    mappedList = newMappedList;
                }
            }
            mappedList.add(value);
            cachedMap.replace(mapListKey, mappedList);

            cache.put(
                cacheKey,
                cachedMap
            );

        }
        return value;
    }



}

4

1 に答える 1

2

複数のスレッドが特定の に書き込むことができる場合List( をList<Integer>追加intしているため、これは である必要があります)、何かを同期する必要があります。ただし、そのグローバル ロックは必要ありません。さらに、あなたはそれを考えて、あなたがそれらに入れたオブジェクトをコピーして、それらから取得したように見えます。なぜなら、それらが更新されたら、それらを再び置くからですCache。ConcurrentHashMap

add()この方法を次のように変更します。

public Integer add(int cacheKey, String mapListKey, int value) {
    // You don't need to synchronize here, since the creation of the map is not
    // synchronized. So either it has been created before, or it hasn't, but there
    // won't be a concurrency problem since Cache is thread-safe.
    ConcurrentMap<String, List<Integer>> cachedMap = cache.getIfPresent(cacheKey);
    if (cachedMap == null){
        // We DON'T want to create a new map automatically if it doesn't exist
        return null;
    }

    // CHM is of course concurrent, so you don't need a synchronized block here
    // either.
    List<Integer> mappedList = cachedMap.get(mapListKey);
    if (mappedList == null) {
        List<Integer> newMappedList = Lists.newArrayList();
        mappedList = cachedMap.putIfAbsent(mapListKey, newMappedList);
        if (mappedList == null) {
            mappedList = newMappedList;
        }
    }

    // ArrayList is not synchronized, so that's the only part you actually need to
    // guard against concurrent modification.
    synchronized (mappedList) {
        mappedList.add(value);
    }

    return value;
}

実際、CacheofLoadingCacheの代わりに of をCache作成するConcurrentHashMapと、コードがadd()よりシンプルになり、 の作成が実装に移動しListますCacheLoader。メソッドを使用して s をLoadingCaches として公開することもできます。ボックス化/ボックス化解除もいくつか削除しました。MapasMap()

EDIT : の戻り値の型を、オリジナルでは機能しないのではなく に変更しましたadd()(戻りboolean値の型が の場合)。潜在的な NPE は必要ありません。intreturn nullInteger

public enum MyCache {
    INSTANCE;

    private static Cache<Integer, LoadingCache<String, List<Integer>>> cache =
            CacheBuilder.newBuilder()
                    .maximumSize(1000)
                    .build();

    private static AtomicInteger uniqueCount = new AtomicInteger(0);

    public int newMapItem() {
        int key = uniqueCount.incrementAndGet();

        //We dont care if something exists
        cache.put(key, CacheBuilder.newBuilder().build(ListCacheLoader.INSTANCE));

        return key;
    }

    public void expireMapItem(int key) {
        cache.invalidate(key);
    }

    public boolean add(int cacheKey, String mapListKey, int value) {
        // You don't need to synchronize here, since the creation of the map is not
        // synchronized. So either it has been created before, or it hasn't, but there
        // won't be a concurrency problem since Cache is thread-safe.
        LoadingCache<String, List<Integer>> cachedMap = cache.getIfPresent(cacheKey);
        if (cachedMap == null) {
            // We DON'T want to create a new map automatically if it doesn't exist
            return false;
        }

        List<Integer> mappedList = cachedMap.getUnchecked(mapListKey);

        // ArrayList is not synchronized, so that's the only part you actually need to
        // guard against concurrent modification.
        synchronized (mappedList) {
            mappedList.add(value);
        }

        return true;
    }

    private static class ListCacheLoader extends CacheLoader<String, List<Integer>> {
        public static final ListCacheLoader INSTANCE = new ListCacheLoader();

        @Override
        public List<Integer> load(String key) {
            return Lists.newArrayList();
        }
    }
}
于 2012-09-13T08:17:41.400 に答える