fix: 当config.isExpireAfterAccess() 为false时,无需加锁#952
fix: 当config.isExpireAfterAccess() 为false时,无需加锁#952WangFeiCoder wants to merge 1 commit intoalibaba:masterfrom
Conversation
|
wangfei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| }finally { | ||
| lock.unlock(); | ||
| } | ||
| holder.setAccessTime(now); |
There was a problem hiding this comment.
放进锁的目的为为了什么呢,当config.isExpireAfterAccess() 为false时,判断逻辑不会走到,加一个锁 仅为了赋值吗。代码中锁加在else 并没没有整个方法,应不会影响其他获取holder对象属性吧。或者是有地方获取holder时加了锁?
There was a problem hiding this comment.
对accessTime的读和写必须都在锁内,如果放在锁外,那么对accessTime的写入操作不能保证被别的线程看见。
此外,如果config里面的expireAfterAccess和expireAfterAccessInMillis需要在运行时间动态调整的话,光使用这里这个锁是不管用的,只能实现不严格的最终可见,目前就只打算做到这样。
There was a problem hiding this comment.
其实,要增加效率,这个锁就应该去掉,把这些内容放到子类LinkedHashMapCache/CaffeineCache里面去处理,就不用额外加锁了,但是要改涉及到的东西也有点多(放在AbstractEmbeddedCache里面当时主要是想在父类里面统一处理一些逻辑,既然这个逻辑下沉到子类的,其它相关逻辑是不是也要下沉到子类),由于当前定义的InnerMap不能返回错误码,只能用返回null代替,那么最终返回的EXPIRED就会变成NOT_EXISTS,行为略有改变,一些单元测试可能会失败。或者,就要修改InnerMap的定义。
|
这个expireAfterAccess没有被完整实现,只在local cache有实现,而且实现很简陋。所以在上层api都没有暴露这个功能,一般人不会用到。 我现在也懒得大改,要是觉得这个锁对于没有用到它的功能碍事,可以像你这样简单改了,但其实会有一些细节问题。 |
当config.isExpireAfterAccess() 为false时,无需加锁