gaogaotiantian / biliscope

Bilibili chrome extension to show uploader's stats
MIT License
594 stars 46 forks source link

Remove avid and bvid from wordMap #121

Closed F-park closed 9 months ago

F-park commented 10 months ago

完成的事情

Resolve #112

gaogaotiantian commented 10 months ago

这个pass current user name的方式有点太危险了。虽然biliscope确实没有非常努力地减少global variable,但是只为了current user name做一个variable进行传参还是有点太overkill了。这个变量本身的变化频率太高了,而且会有一个sync的问题(用户名是有可能晚于wordMap被传过来的),这样感觉是无谓地增加了程序的复杂程度,大概率会引入更多的bug。

现阶段先避开用户名这个事情,只去做av和bv吧。

F-park commented 10 months ago

这个pass current user name的方式有点太危险了。虽然biliscope确实没有非常努力地减少global variable,但是只为了current user name做一个variable进行传参还是有点太overkill了。这个变量本身的变化频率太高了,而且会有一个sync的问题(用户名是有可能晚于wordMap被传过来的),这样感觉是无谓地增加了程序的复杂程度,大概率会引入更多的bug。

现阶段先避开用户名这个事情,只去做av和bv吧。

用户名并不会晚于写入词云后再刷新,我改变了前后逻辑,一定会先刷新用户名再写入词云的。

biliGet(`${BILIBILI_API_URL}/x/space/wbi/acc/info`, {
    mid: userId,
})
.then((data) => {
    cacheAndUpdate(callback, userId, "info", data);
    // 这上面的代码会刷新 currentUserName
    // 函数调用链 cacheAndUpdate -> callback (实际上调用的是 userProfileCard.updateData) 

    updateRelation(userId, callback);

    updateVideoData(userId, callback);
    // 这上面的代码会调用 updateWordMap 函数
    // 函数调用链 updateVideoData -> requestSearchPage -> updateWordMap
})
gaogaotiantian commented 10 months ago

我只是举了个例子。而且你这样相当于把一个可以parallel进行的过程变成线性的了吧,这是增加了数据出现的latency的。这里用global variable传参是个明确不好的方式,而且收益有点低了。

F-park commented 10 months ago

我只是举了个例子。而且你这样相当于把一个可以parallel进行的过程变成线性的了吧,这是增加了数据出现的latency的。这里用global variable传参是个明确不好的方式,而且收益有点低了。

就一个请求异步改同步,我觉得应该不会拖慢太多速度吧。 如果一个用户连用户空间详细信息都加载不出来,那我觉得也没必要加载 RelationVideoData 了。


现在 currentUserName 不再是 global variable 了。

gaogaotiantian commented 9 months ago

你这个对性能的观点也是波动比较大。之前会觉得一个函数调用了多次想改成一次,但是那个调用可能是微秒级别的。这个几十毫秒甚至百毫秒的,影响渲染时间的delay倒是觉得没多少了……

我们最终目的是ABC都渲染出来,你现在变成了,你需要先拿到A的值,再去拿B和C。而我们完全可以ABC一起去拿。没人说A是拿不到的,只是我们这个时间明明可以请求BC,为啥要等着呢?

现在的currentUserName本质上也是个global其实。我的观点是这个事情在大部分up的身上收益太小了。而造成的影响是对每一个up都存在且明显的。和av,bv不一样,这两个的替换是用户感受不到的。

F-park commented 9 months ago

那就删掉吧,之后再看看有没有更好的方式把用户名加进词云吧。