java - 包含列表的缓存 ConcurrentHashMap 的线程安全性

Question

我正在尝试使用 Google Guava Cache 制作 ConcurrentHashMaps 的线程安全单例缓存。这些 map 中的每一个都将包含一个列表。该列表只会在所有可以添加到列表中的线程执行完毕后才会被读取一次。我想知道我的实现(特别是我更新项目的地方)是否是线程安全的/如何改进它。有没有更好的方法来做到这一点而不使用同步块(synchronized block)？

公共(public)枚举 MyCache {
    实例;

    私有(private)静态 Cache
    
     > 缓存 =
     
 CacheBuilder.newBuilder()
     
 .最大尺寸(1000)
     
 。 build ();
     

     
 私有(private)静态 AtomicInteger uniqueCount = new AtomicInteger(0);
     

     
 私有(private)最终对象互斥体=新对象()；
     

     
 //创建一个新的唯一ConcurrentHashMap
     
 公共(public)整数 newMapItem(){
     

     
 整数键 = uniqueCount.incrementAndGet();
     

     
 //我们不关心某些东西是否存在
     
 缓存.put(
     
 key ，
     
 新 ConcurrentHashMap
     
      ()
      
 )；
      

      

      
 返回键；
      
 }
      

      
 公共(public)无效expireMapItem(int键){
      
 缓存.invalidate(key);
      
 }
      

      
 公共(public)整数添加(int cacheKey，字符串mapListKey，int value){
      

      
 同步(互斥){
      
 ConcurrentMap
      
        cachedMap = cache.getIfPresent(cacheKey);
       
 if (cachedMap == null){
       
 //如果新 map 不存在，我们不想自动创建它
       
 返回空值；
       
 }
       

       

       
 列表mappedList = cachedMap.get(mapListKey);
       

       
 if(mappedList == null){
       

       

       

       
 列表 newMappedList = new List();
       
 mappedList = cachedMap.putIfAbsent(mapListKey, newMappedList);
       
 if(mappedList == null){
       
 映射列表=新映射列表；
       
 }
       
 }
       
 映射列表.add(值);
       
 cachedMap.replace(mapListKey,mappedList);
       

       
 缓存.put(
       
 缓存键，
       
 缓存 map 
       
 )；
       

       
 }
       
 返回值；
       
 }
       

       

       

       
}
       

      
     
    

Answer 1

如果多个线程可以写入给定的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;
}

<小时/>

实际上，我会创建一个 Cache的LoadingCache s，而不是 Cache的ConcurrentHashMap ，它使 add() 中的代码更简单，感动的创作List到CacheLoader执行。您仍然可以公开LoadingCache为Map使用asMap()方法。我还删除了一些装箱/拆箱。

编辑:更改了 add() 的返回类型至boolean而不是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();
        }
    }
}