search

CyclopsMC / IntegratedTerminals

Terminals for managing and overviewing Integrated Dynamics networks
MIT License
5 stars 6 forks source link

[1.16.5] Looking up a Complex recipe in the terminal results in trying to send a 28MB large packet #80

Closed Speiger closed 2 years ago

Speiger commented 2 years ago

Issue type:

Eh yeah I am back. crash.zip

Someone tried to look up a HV Solar Panel recipe, which is like 8 layers deep, and roughly 2-3k crafts when you try to resolve all its requirements. And it caused a server crash because the Terminal Tried to send a 28MB packet.

Why exactly do you need to send 28 MB of data that seems to be a entire serialization of the context?

Have fun reading through that.

rubensworks commented 2 years ago

@Speiger Your issue appears to be not following one of the allowed issue templates, which breaks our automation tools. Please update your issue to the proper template.

Speiger commented 2 years ago

Ok fixed.

rubensworks commented 2 years ago

Thanks!

Speiger commented 2 years ago

I wouldn't close this issue, because I doubt its fixed yet, because you only fixed the "symptom" instead of the actual issue.

Ok, here comes my 5 cents to that patch.

I doubt this fixes the major issue. You removed 1 point of data that is an issue, but you are actually not compressing enough data, unless this 1 variable was 90% which I highly doubt it.

Why? Because the concept itself isn't compressed at all. You are still manually saving every single required recipe, recursively.

That means if a recipe is required duplicate you are still storing the recipe duplication's too. So if you have 10 different recipes and all of them combined are required like 10k times. Instead of counting which recipe is required how many times you are manually saving each instance. A "translation" name even if it is really long shouldn't represent 26 MB, even if you turn a 200 bytes string (compressed) into a int you are saving 196 bytes. And since we have 5k crafts (worst case estimation) you are only saving 1MB of the issue. Well 25 MB to go.

I will try the patch and force the crash to happen again.

rubensworks commented 2 years ago

PRs for further compression are certainly welcome. Since my available time for development is very limited, I focus on the most urgent problems first.

Speiger commented 2 years ago

Nope don't have the time to invest into a full rewrite of a system. I still have to test / maintain 3 mods that are actively being updated right now.

I just threw out a warning Integrated dynamics is not allowed to be used for crafting to complex things. Either Use IC2C or RS, way safer on the server.

One suggestion I could give you. Turn NBT keys into 1-2 letters and use that. Saves you a lot more.

Also maybe add a profiler for you in so you can actually see how much data a simple recipe eats. (I have one for every single item/block that i have so i can optimize things like item storages from 2kb of a normal double chest down to 200 bytes, etc etc)