univrsal / JustEnoughButtons

Adds utility buttons to the inventory screen
Mozilla Public License 2.0
6 stars 5 forks source link

Lag with magnet #23

Closed LemADEC closed 7 years ago

LemADEC commented 7 years ago

On a good dedicated server with a single player online and having magnet on, using justenoughbuttons-1.10.2-1.7.1, we can observe 3% CPU load just due to magnet handling: image

univrsal commented 7 years ago

I ran a profiler on a local server and I couldn't even find the event calls but there's no doubt that my code is inefficient. But I can't think of any other approach for the magnet mode, the only thing that I could think of that would help with performance is a delay between the magnet mode calls.

LemADEC commented 7 years ago

I was checking an area after removing tree logs, so there was a lot of saplings and sticks on the floor. getEntitiesWithinAABB() is known to be badly implemented. Since there's generally way more entities than players, it's probably more efficient to inverse the scan into something like:

for all worlds
   if no listed players, then skip
   for all entities
      if not entityItem, then skip
      for all players
         xxxx
univrsal commented 7 years ago

Are you sure that iterating over all entities in a world would be faster than only getting the ones in a predefined area? Botania for example is doing it the same way only with a delay. So I'd guess that I should just check for items like every 10th event call

LemADEC commented 7 years ago

We need to reduce the calls to getEntitiesWithinAABB(), either do it less frequently like Botania, or re-implement the latest. If you choose to do the former, remember you need to also give a motion to the item instead of just setting it's position since entities will fall during 2 updates.

univrsal commented 7 years ago

Alrighty I'll see if I can figure out the profiler to test out both ways

univrsal commented 7 years ago

So I added a line to reduce the calls of getEntitiesWithinAABB and reduced the priority of the Subscribe event. The magnet mode still works fine for me so I don't see a need to add velocity, since it's still picking up items pretty consistently.

After some fiddling to figure out the profiler it seems to me that the cpu usage is now pretty insignificant:

unbenannt

But since I'm not very familiar with profiling, you might wanna give the new build another try.

LemADEC commented 7 years ago

will do

univrsal commented 7 years ago

Since I didn't get any response I'll just close this for now