xfangfang / wiliwili

第三方B站客户端,目前可以运行在PC全平台、PSVita、PS4 、Xbox 和 Nintendo Switch上
https://xfangfang.github.io/wiliwili
GNU General Public License v3.0
3.18k stars 146 forks source link

[wip] image help 去锁优化 #139

Closed dragonflylee closed 1 year ago

dragonflylee commented 1 year ago

观察了一下 ImageHelper::with ImageHelper::clean 的调用都在主线程,所以对于 requestPool requestMap 的操作都在主线程就不需要加锁了

实际上 cpr::threadpool 已经给工作队列加锁了

  1. requestPool 比起请求队列更像是个对象池
  2. share_ptr<ImageHelper> 直接 bind 到请求线程上,请求线程未结束的时候对象不会释放,问题在于切换到 brls::sync 时候 ImageHelper 可能已经被释放掉了
  3. requestMap 的 key 可以用来防止重复删除对象,可以解决2 的问题
xfangfang commented 1 year ago

@dragonflylee 感谢花时间帮忙梳理这部分的代码,这一部分之前写的比较乱,辛苦啦!

目前发现有一处小问题,在上面评论出来了,更新的话可以顺便修改一下borealis的版本,貌似之前的 ShellExecute 没办法在uwp下正常编译,我做了一点小的修改。

dragonflylee commented 1 year ago

uwp 那个我正在找资料看 似乎使用的是其他api

dragonflylee commented 1 year ago

另外 126行那里的判断仍然不是严谨 有可能对象刚放到池子里又被别人用了

此时请求刚返回 就不对了 虽然不会崩溃

xfangfang commented 1 year ago

目前的网络图片访问在同一时间多次请求同一张图片时,会同时去请求,哪个请求先结束用哪个。 这个问题会出现在评论区加载多个相同的表情包时,这个影响不是很大,但是是一个可以优化的点。

dragonflylee commented 1 year ago

目前的网络图片访问在同一时间多次请求同一张图片时,会同时去请求,哪个请求先结束用哪个。 这个问题会出现在评论区加载多个相同的表情包时,这个影响不是很大,但是是一个可以优化的点。

ImageHelper::with 都是工作在主线程,肯定有先后,最粗暴的方法就是再搞一张 url 做key 的hash表

xfangfang commented 1 year ago

另外 126行那里的判断仍然不是严谨 有可能对象刚放到池子里又被别人用了

此时请求刚返回 就不对了 虽然不会崩溃

之前我的做法是在 ImageHelper::clear 里取消请求,然后等到 ImageHelper::requestImage 中断请求或者请求结束再放回 requestPool 不知道为什么这里要修改呢?

dragonflylee commented 1 year ago

另外 126行那里的判断仍然不是严谨 有可能对象刚放到池子里又被别人用了 此时请求刚返回 就不对了 虽然不会崩溃

之前我的做法是在 ImageHelper::clear 里取消请求,然后等到 ImageHelper::requestImage 中断请求或者请求结束再放回 requestPool 不知道为什么这里要修改呢?

解决并发问题一般有两种做法,一种是加锁,还有一种就是将对同一对象的操作放到同一线程。去掉锁理论上性能更好

另外这个问题之前也有,当主线程调用 ImageHelper::clear 的时候,刚好请求也顺利结束正在加载图像,之后 sync就排在 ImageHelper::clear 的后面执行,(其实把 cancel 判断提前也能解决大部分情况的问题)

好像修改前后本质上也没啥改变,就是去掉了锁

xfangfang commented 1 year ago

感谢解释,我的理解是这样的:

之前存在的问题是,有可能clear不生效,需要在 requestImage 中的 sync 里判断一下cancel。

现在的问题是,如果clear之后,请求还没结束,ImageHelper 就被加入 requestPool 里了,紧接着有大量的图片请求,同一个 ImageHelper 重新被绑定到新的图片组件,因为这个时候 isCancel 变成 true 了,新的图片组件可能会出现短时间显示错误的图片的问题。

但是现在的调用方式,很难让这两个问题出现。