Memory Leak in current master branch

[Soundofdarkness]

Seems that we currently have a memory leak happening when swapping away from a page with an item set button.
I haven't found any obvious cause, so far, I just hope its not some internal electron bug as it seems to not be disposing of state correctly.
(Just going to move this from PR #117 to here, in case somebody has an idea)

[Soundofdarkness]

Alright, found the issue I think. Seems like accessing the freezer for bigger objects inside tags causes it. (In this case itemsinfo)
I've "fixed" (more like test-patched) the memory leak itself, via just handing in the itemsinfo as static opts (which pretty sure breaks the language functionality quite a bit), and now it just causes a massive cpu load (completely pins 1 core to 100% on my cpu), which well, isn't exactly better. I guess that happens based on how riotjs handles updating tags with really big elements, so we might need to add the language info to the item set directly ?

Edit again: Ram issue is back suddenly, no idea why but it seems like what I thought at first is wrong. Unfortunately I don't have more time today, so I'm probably going to continue looking into this in the next days.

[89Q12]

I took a quick look at it and I couldn't really reproduce it, yeah RuneBook uses a bit more when different pages get loaded but nowhere near a gig. Maybe its because I'm on Linux or so I don't know. Anyways I will take a deeper look at it during the weekend

[HannoMartinus]

I can confirm it happened to me too (twice), but I didn't investigate it further.

[ghost]

It could be a problem with RiotJs. We are also using a very old version (3.13.2 vs 6.1.2). We need to throw out riot-i18n and replace it with something else. Then we could also update RiotJs and possibly get less problems.

[89Q12]

True, I could put some time into updating everything but I can't confirm that I didn't in fact introduce this myself.
It could that I overwrite the item info in the freeze and not set it null beforehand. That's the only thing I can think of where the leak could come from as for the opgg rework I checked for that and other issues before it got merged.
Hope this makes somewhat sense.

[ghost]

But there are bugs in the item explorer in general (since I don't use it I didn't deal with it).

• Open Runtbook from VS code
• Select u.gg and search for any champ

The console is spammed with:

Cannot read property 'start_items' of undefined
Cannot read property 'core_items' of undefined
Cannot read property 'big_items' of undefined

Maybe there is an error here. Btw. I'm thinking about rewriting Runebook in C# this whole Electron/JS crap is just not mine^^

[89Q12]

Yeah that's because I messed up an if statement in page-list it needs to be if={page.name == "bltitzgg" || page.name == "opgg"} and what's currently is^^ and sounds doable but it will take you a lot of time

[Soundofdarkness]

I'm personally guessing its something related to RiotJS too, as I've tried storing the items data in localstorage instead of the freezer, but still had the same problem. Problem with RiotJS is that they dont recommend migrating from v3 to higher versions since 3.x is still maintained according to them (seems like they changed quite a bit from v3 to v6).
So far all of this sounds a lot like there wont be a good solution to fixing this.

@Rerago Problem with C# is just going to be cross platform UI, since iirc there is still just Avalonia. Not sure if that works well enough or not. Aside from that it sounds like an idea, just going to be a ton of work.

[89Q12]

Okay, thanks for the information while I would like to work on an update I can't because my finals are coming up :/ But in around 35 Days I would be able to take a look at it again if nobody updated to a newer version by then.

[Soundofdarkness]

Sounds good and don’t worry ! Finals are way more important, take all the time you need. I’m still looking into it too in my free time, but since I have a lot of health stuff going on in my family I’m limited too.

[89Q12]

Okay so I did some testing on a macbook(M1) and suprise suprise I don't ever see a spike nor do I see high usage in general the usage is around 90Mb(Task manager). Very strange

I through all plugins and opened a few itemsets:

[Soundofdarkness]

Alright,
then I'm going to test it again on a new system later, in case it was some weird issue on that windows install.

[Soundofdarkness]

Okay, I quickly tested it again too, now it seems to work for me too.
Strange, but well, sounds good. I'm going to test it for around a day more, but seems like we maybe don't need to revert it.

[Soundofdarkness]

@HannoMartinus Since you had the issue too at some point: Would you maybe be able to check if the issue persists for you ?
As if its gone for you too I would feel more comfortable closing this as the decision would not be completely dependent on my Windows install then.

[Soundofdarkness]

Okay, now I had the bug yet again, but it seems like it only happens with dev tools open ? Which could actually make sense since it seems to try to inspect that 1MB json from op.gg and that even multiple times, so maybe thats just something it can't deal with.
As soon as im doing the exact same without dev tools open it works perfectly fine, same as in release mode (aka just built the executable and ran that). So I guess we can probably release it, and it should be okay. If not I guess we can only un-release the new version and tell people to download the old one.