Forgery

Forgery

823k Downloads

FabConf.isEnabled performance

CryingNova opened this issue ยท 4 comments

commented

I apologize, but I'm not too sure if this is an issue exactly on its own or an incompatibility. That's why I've included both the debug requested and one of the many Spark profilers I've been conducting over the past few days.

I should note I only recently updated Forgery to its newest version, so I've only recently experienced this issue with the mod. Currently using version: forgery-3.3.11+1.16

Players have been complaining a bit more than usual about lag and I typically just start a profiler and look at logs to narrow down the issue. This may be due to an incompatibility between Forgery and Ice & Fire? Specifically referencing net.minecraft.entity.Entity.fabrication$getUpstream() under the Ice & Fire tree of the Spark profiler.

Nearly all of the other profilers I have taken referenced Forgery and Ice & Fire in that exact order, so I am assuming it's part of the reason for some new lag.

https://spark.lucko.me/jCV573xUCr

https://gist.github.com/CryingNova/7f1c0009c5c0b36dd7051f48efcf6558

commented

I'll answer since I'm more performance-minded, despite not really working on Fabrication anymore.

Entity fluid ticking (one of a handful of methods that has changed names a few times between 1.16 and 1.20) is an extremely hot codepath, and the entities_sink_in_falling_fluids feature patches a method called in it:

Even if the option is disabled, you pay for the FabConf.isEnabled lookup. However, looking at your profile, this is only ~6% of your CPU time โ€” where 25% of it is spent simply waiting for time to pass, as your server is not lagging.

Profiles taken on a normally-performing server are notoriously unreliable due to sampling bias and just plainly not having enough data. However, it's been known for a while that isEnabled's performance is subpar โ€” but there's not a lot to be done to it, other than using a FastUtil Object2ObjectMap rather than a vanilla HashMap.

image

Of the hot calls to isEnabled, though, getUpstream is only the second most common.

You will likely want to set entities_sink_in_falling_fluids and collision_based_landing_pos to Banned, to prevent them from loading their mixins and causing the performance hit. Again, though, this profile is not conclusive.

Ice&Fire is likely exacerbating this by using complex entities โ€” that is, entities that are made up of a bunch of smaller entities all tied together, like the Ender Dragon. Each of those parts needs to individually tick, perform collision, and indeed, check their fluid state.

commented

if you don't regularly change features using general.limit_runtime_configs is advised as it almost entierly eliminates the cost of isEnabled.

commented

omg these 3 things helped A TON. Server is now running just as smooth as before, even when under a lot of stress.

Thank you for the very insightful information and I'll surely keep it in mind for the future! <3

commented

Also i don't know how applicable this is since i haven't tested it at all.
but let's say you wanted to have entities_sink_in_falling_fluids but it's causing a heavy performance hit. you could probably turn on taggable players and run /fabrication tag push *.entities_sink_in_falling_fluids untagged_players_only to stop the feature from running on non player entites and possibly improve performance? (while having the feature for players at least)