-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: 当config.isExpireAfterAccess() 为false时,无需加锁 #952
base: master
Are you sure you want to change the base?
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. |
} | ||
holder.setAccessTime(now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这一行得进锁里面去。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
放进锁的目的为为了什么呢,当config.isExpireAfterAccess() 为false时,判断逻辑不会走到,加一个锁 仅为了赋值吗。代码中锁加在else 并没没有整个方法,应不会影响其他获取holder对象属性吧。或者是有地方获取holder时加了锁?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
对accessTime的读和写必须都在锁内,如果放在锁外,那么对accessTime的写入操作不能保证被别的线程看见。
此外,如果config里面的expireAfterAccess和expireAfterAccessInMillis需要在运行时间动态调整的话,光使用这里这个锁是不管用的,只能实现不严格的最终可见,目前就只打算做到这样。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
其实,要增加效率,这个锁就应该去掉,把这些内容放到子类LinkedHashMapCache/CaffeineCache里面去处理,就不用额外加锁了,但是要改涉及到的东西也有点多(放在AbstractEmbeddedCache里面当时主要是想在父类里面统一处理一些逻辑,既然这个逻辑下沉到子类的,其它相关逻辑是不是也要下沉到子类),由于当前定义的InnerMap不能返回错误码,只能用返回null代替,那么最终返回的EXPIRED就会变成NOT_EXISTS,行为略有改变,一些单元测试可能会失败。或者,就要修改InnerMap的定义。
这个expireAfterAccess没有被完整实现,只在local cache有实现,而且实现很简陋。所以在上层api都没有暴露这个功能,一般人不会用到。 我现在也懒得大改,要是觉得这个锁对于没有用到它的功能碍事,可以像你这样简单改了,但其实会有一些细节问题。 |
当config.isExpireAfterAccess() 为false时,无需加锁