MikeLydeamore / Ex-Astris

Ex-Nihilo Addon
9 stars 9 forks source link

AutoHammer killing game #7

Closed Slind14 closed 9 years ago

Slind14 commented 9 years ago

Hey,

only a few auto hammers are already killing the performance: image

Slind14 commented 9 years ago

https://github.com/MikeLydeamore/Ex-Astris/pull/8

trading a little bit of mem for performance. VisualVM doesn't even show the autohammers anymore with the changes, so a screenshot form it would be unnecessary

MikeLydeamore commented 9 years ago

This is intriguing, considering that both the hammer registry and the sieve registry use the same system for checking if items are registered (see https://bitbucket.org/Erasmus_Crowley/ex-nihilo/src/a31e934bdf2bc29a5b8ac9b6dff2b938c2e21d3b/src/main/java/exnihilo/registries/HammerRegistry.java?at=master and https://bitbucket.org/Erasmus_Crowley/ex-nihilo/src/a31e934bdf2bc29a5b8ac9b6dff2b938c2e21d3b/src/main/java/exnihilo/registries/SieveRegistry.java?at=master for examples there), and the system inside both the AutoSieve and the AutoHammer is nearly identical. The exception in Ex Nihilo source is the string comparison. It's late here, and I will look over this in the morning to make sure.

Slind14 commented 9 years ago

I had about the same amount of auto sieves as auto hammers, so the snapshot should be accurate.

Slind14 commented 9 years ago

Edit: this one is probably the reason: reward.source.getUnlocalizedName().equals(block.getUnlocalizedName()) https://bitbucket.org/Erasmus_Crowley/ex-nihilo/src/a31e934bdf2bc29a5b8ac9b6dff2b938c2e21d3b/src/main/java/exnihilo/registries/HammerRegistry.java?at=master#cl-115 https://bitbucket.org/Erasmus_Crowley/ex-nihilo/src/a31e934bdf2bc29a5b8ac9b6dff2b938c2e21d3b/src/main/java/exnihilo/registries/SieveRegistry.java?at=master#cl-72

Slind14 commented 9 years ago

anyways you might still want to move the slot check before the registry at all places: https://github.com/MikeLydeamore/Ex-Astris/blob/master/src/main/java/ExAstris/Block/TileEntity/TileEntitySieveAutomatic.java#L474

In the end I tripled the server performance so I'm happy with the caching result.

MikeLydeamore commented 9 years ago

More on this to come, will be moving the AutoSieve to the same system.

Slind14 commented 9 years ago

@MikeLydeamore Hey, after running with the caching for some time I noticed that this issue should be addressed in ex nihilo it self. While it isn't such a big deal there it is still noticeable. Do you know the ex nihilo dev and think you could convince him to build in something similar or copy it over? (The string.append and Hashtable.get are the main source of the performance issue here)

MikeLydeamore commented 9 years ago

The issue does indeed lie inside Ex Nihilo, but there it is rarely an issue as the registry isn't checked anywhere near as often as I do it. Also, a lot of the performance problems were coming from the fact I was checking the registry before checking the slot - so you were getting 22 checks instead of 1. I have mentioned this to Crowley previously, but I understand he is very at the moment and so the update times are slow.

Slind14 commented 9 years ago

Yeah, I noticed this with the slots and am also aware that it is by far less in the way he does is it than you did. But as lava is the main power source and 100 crucibles belonging to a normal setup, well it is noticeable.

MikeLydeamore commented 9 years ago

Crucibles I believe use a HashTable, and so the lag for producing a lava with lookups should be minimal. The sieve, by the way, used the same system as the autohammer, and never had any complaints about performance, so I expect it was more the incorrect slot order.

Slind14 commented 9 years ago

yeah hashtable + string append. those two end up pushing it to +1%. And 1% is a lot as zombies are at 0.6% as comprehension.