代码求优化（单例）

Question

大侠们帮我看看这个单例有没有多线程的问题
public class CacheData {
private static Logger log = Logger.getLogger(CacheData.class);
private Lock mapLock = new ReentrantLock();
private Map<String, List<ChannelState>> ipChMap = null;

private volatile static CacheData instance = null; private CacheData() {

getData();
log.debug("1: ipChMap size:" + ipChMap.size());
} public static CacheData getInstance() {
if (instance == null) {
synchronized (CacheData.class) {
if (instance == null) {
instance = new CacheData();
log.debug("getInstance ");
}
}
}
return instance;
} private void getData() {
mapLock.lock();
try {
Map<String, List<ChannelState>> temp = new HashMap<String, List<ChannelState>>();
List<ChannelState> csList = new ChannelStateDao().queryAll();
for (ChannelState cs : csList) {
String ip = cs.getIp();
List<ChannelState> ls = new ArrayList<ChannelState>();
ls.add(cs);
if(temp.containsKey(ip)){
temp.get(ip).add(cs);
}else{
temp.put(ip, ls);
}
}
log.debug("getData:" + csList.size());
ipChMap = temp;
} finally {
mapLock.unlock();
}
} public List<ChannelState> getChannelStateByIp(String ip) {
log.debug(ip);
mapLock.lock();
try {
List<ChannelState> ls = null;
ls = ipChMap.get(ip);
return ls;
} finally {
mapLock.unlock();
} } public void update() {
CacheData.getInstance().getData();
}}

Answer 1

[code=Java]
public static CacheData getInstance() {
        if (instance == null) {
            synchronized (CacheData.class) {
                if (instance == null) {  //这一行是做什么的！前面不是判断过了么
                    instance = new CacheData();
                    log.debug("getInstance ");
                }
            }
        }
        return instance;
    }
[/code】
没看出来有什么问题！对锁了解不深。

Answer 2

private Map<String, List<ChannelState>> ipChMap = null;这种数据结构看着就非常恶心！用什么双检锁啊，真费电，直接在静态字段上 new 吧。这种双检锁几乎就是杞人忧天，new 一个对象分配内存并没有你想像得那么耗费资源，这种性能损失可以忽略不计！

Answer 3

双向检查模式不是早就证明不能应用到java多线程中吗(具体参考effectiv java)？
简单点就直接
 public synchronized  static CacheData getInstance() {
       if (instance == null) {
            instance = new CacheData();
            log.debug("getInstance ");
        }
        return instance;
 }
效率有也不会有多大影响的，因为一次instance初始化以后，其他线程得到锁会很快释放锁。这点性能的损失可以忽略不计。

Answer 4

    public List<ChannelState> getChannelStateByIp(String ip) {
        log.debug(ip);
        mapLock.lock();
        try {
            List<ChannelState> ls = null;
            ls = ipChMap.get(ip);
            return ls;
        } finally {
            mapLock.unlock();
        }    }我认为这段代码不需要用锁的，因为不会出现线程的冲突，用了锁反而会使程序的
效率变低。

Answer 5

个人觉得getInstance()方法改成如下形式或许更直观简洁点：
public synchronized static CacheData getInstance() {
   if (instance == null) {
       instance = new CacheData();
       log.debug("getInstance ");
   }
   return instance;
}
既然CacheData要搞成单例的，那么，该单例类或getInstance()方法是否定义为final类型的更合适些？！请大伙儿指正！

Answer 6

    private void getData() {
        mapLock.lock();
        try {
            //Map<String, List<ChannelState>> temp = new HashMap<String, List<ChannelState>>();
            ipChMap = new HashMap<String, List<ChannelState>>();
            List<ChannelState> csList = new ChannelStateDao().queryAll();
            for (ChannelState cs : csList) {
                String ip = cs.getIp();
                if(ipChMap.containsKey(ip)){
                    ipChMap.get(ip).add(cs);
                }else{
                    // 没有的时候才创建List
                    List<ChannelState> ls = new ArrayList<ChannelState>();
                    ls.add(cs);
                    ipChMap.put(ip, ls);
                }
            }
            log.debug("getData:" + csList.size());
            //ipChMap = temp;
        } finally {
            mapLock.unlock();
        }
    }