Revadike / SteamWebIntegration

Integrate your personal Steam information on the web, at your convenience!
MIT License
146 stars 20 forks source link

Steamtracker fixes; version increment #54

Closed Luckz closed 3 years ago

Luckz commented 3 years ago

ST returns appid as int, which was breaking functionality because we were ===-comparing to a .toString(). I noticed we do a .find on an array here, which is potentially quite slow, so instead I changed it to one-time convert array to object for O(1) lookups and (hopefully 😆) added code to remove old storage if needed.

The dynamicstore-sourced lookups are done using .includes. I assume making a set or object out of all those arrays would make for faster lookups, but might be computationally expensive itself as many people poll a fresh dynamicstore on every single web request.

Luckz commented 3 years ago

BTW, why do we have the setTimeout(() in line 348?

Revadike commented 3 years ago

According to https://jsbench.me/3pkjlwzhbr/1 an object key lookup is indeed the fastest. I was thinking maybe Set would be equivalent in performance, but this is not the case.

Luckz commented 3 years ago

(I was going by https://github.com/anvaka/set-vs-object myself.) It doesn't really matter a lot how much faster a object lookup might be than a set considering in your example the set is already a thousand times (give or take) faster than array includes. The potential extra speedup might be meaningless.

To really squeeze out performance we'd need to see how much effort it is on each page load to

Revadike commented 3 years ago

I think its better to discuss this further on https://github.com/Revadike/SteamWebIntegration/issues/14 Also, I made some changes as well, not sure if you saw it: https://github.com/Revadike/SteamWebIntegration/commit/03abbf46e569b2f054630591492a73800131b2c3

Revadike commented 3 years ago

BTW, why do we have the setTimeout(() in line 348?

I thought I could delete it, but I was wrong. This is added to avoid having the page hang when loading, because it is waiting on our script execution. setTimeout essentially works as a new thread. I added it back.