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（应该是 a List<Integer>，因为你正在向int它添加 s ），你需要同步一些东西。但是，您不需要那个全局锁。此外，您似乎认为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;
}

实际上，我会创建 a Cacheof LoadingCaches，而不是 a Cacheof ConcurrentHashMap，它使代码add()更简单，将创建List移至CacheLoader实现。您仍然可以使用该方法将LoadingCaches 公开为s。我也删除了一些装箱/拆箱。MapasMap()

编辑：将返回类型更改为add()toboolean而不是int原始类型不起作用return null（当返回类型为 时Integer）。不需要潜在的 NPE。

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 回答