Ellpeck / ActuallyAdditions

A Minecraft Mod about lots of useful gadgets and things!
http://minecraft.curseforge.com/projects/actually-additions
MIT License
212 stars 96 forks source link

1.12.2 - Requesting input from maintainer for future laser relay reverse-lookup optimization PR #1397

Open ByThePowerOfScience opened 6 months ago

ByThePowerOfScience commented 6 months ago

I'll probably be the one to implement anything that comes from this, so don't worry about extra effort coming from replying to this. I just want to get the input of someone who knows the codebase better than I do before making my PR.

Problem Location: LaserRelayConnectionsHandler#getNetworkFor

So the issue is that getting the connections for a given blockpos happens multiple times per tick on the server side and multiple times per frame on the client-side. Each time a laser needs to know its network, it has to iterate through the entire ConcurrentSet to find its own position, which easily takes up over 13% of client time in my base and ~1.5% server time on my server.

I'm thinking up a Mixin softpatch for it in the modpack I work on, but I wanted to raise the issue here to see if you could think of any better way to handle this.

The logic currently is something like this:

Possible strategies:

I feel like the reverse-lookup map is the best idea, but I wonder if we could go even further and only store the networks in the reverse-lookup map? Idk, I'll probably just save the frames at the cost of memory in my softpatch, but for the official PR I want something more coherent.

Anyway, thank you for your time!

Ellpeck commented 3 months ago

yea the code sucks and is from when i was like 15 and had no idea about anything lmao, it could definitely use with some redesigning in terms of that. please note that 1.12.2 is not supported anymore and won't have any pull requests merged, though, so if anything, this is a 1.20.4+ update :)

using a bidirectional map (i think java has a builtin type for this, or at least one of the many libs that minecraft uses does) is the easiest solution, as using a graph would mean including a third-party library (like i do in pretty pipes, it's a PAIN to maintain) or writing a good algorithm yourself which, again, is pretty error-prone and probably takes some maintenance work!