Ellpeck / PrettyPipes

Pretty Pipes is a simple to use, all-inclusive item transport mod featuring simple pipes that can be upgraded using modules to accomplish much more advanced tasks.
https://modrinth.com/mod/pretty-pipes
MIT License
21 stars 19 forks source link

Fix performance issue in getOrderedNetworkItems() #205

Closed Gieted closed 6 months ago

Gieted commented 6 months ago

I've noticed that my server is having low tps problem, despite good hardware. After some debugging, I've concluded that lag is caused by PipeNetwork.getOrderedNetworkItems() function.

A simple change could be made to improve its performance and fix the lag.

Ellpeck commented 6 months ago

Hi, thanks so much for this pull request! Please fix any compile issues and I can merge :)

Ellpeck commented 6 months ago

Implementing a version of this myself. Thanks for the pull request, though! ❤️

Gieted commented 5 months ago

@Ellpeck Sorry for not responding, I was pretty busy last week.

I've looked into your changes on master and have 2 suggestions:

  1. Here I've used LinkedHashMap instead of HashMap, because regular HashMap might scrable order of elements, while LinkedHashMap keeps the order of addition. Not sure if it's relevant to your implementation.

  2. It depends on specific implementation of Map, but putIfAbsent() might actually be faster than containsKey() (on the other hand, you are skipping an unnecessary object creation, so it isn't a clear-cut which one will be actually faster in practice).

Ellpeck commented 5 months ago

Hm, yea, I meant to also be using the linked hash map but I guess I must've forgot about it halfway through. It seemed to work from my testing, but I know that's largely because element order is preserved most of the time, just not guaranteed to be. Thanks for flagging that up!