Repurposed Structures (Quilt/Fabric)

Repurposed Structures (Quilt/Fabric)

8M Downloads

Mod using a lot of resources (5% total) even for already generated chunks.

MarkusBordihn opened this issue · 6 comments

commented

I like the mod, but unfortunately it cause a high load (5%) even for already generated chunks compare to other mods.

How to reproduce:

  • Install Repurposed Structures and Spark or use any similar profiling tool
  • Start the game and start profiling over /spark profiler
  • Move around the world
  • Stop the profiler over /spark profiler --stop and view the results

It will show something like:

com.telepathicgrunt.repurposedstructures.misc.MobSpawningOverTime.getStructureSpawns() 3.28% 1038.18ms (self: 223.12ms - 0.70%) (repurposed_structures)
com.telepathicgrunt.repurposedstructures.misc.MobSpawningOverTime.getStructureSpawns() 1.93% 770.99ms (self: 207.23ms - 0.52%) (repurposed_structures)

Example Profile: https://spark.lucko.me/DgDd4dJrYG?hl=71,343

Please try to optimize the related functions, I would expect that the load for an already generated chunk should be below 1% and not 5% total.

commented

Thanks for your answer, but unfortunately I need to disagree in some points with your answer, but let me clarify my point.

You are using a lot of mixin's and core modding. I would not really say it's the normal behavior of how vanilla works otherwise you could just use the standard api's and don't need to use your own implementations.
Furthermore you bypass some of the standard forge api's and event's, so even if there is a mod which optimizes the structures or mob spawn it would not have really any positive effect on your mod which are doing stuff outside of these api's or the vanilla behavior.

Anyways I'm not knowing the reasoning behind these design decision and not criticism them, but something looks wrong for me.
I even get this 5% load by just standing still in a already loaded chunk and blocking all of the standard mob spawning events.

In a loaded chunk with an active player, there is normally no need to re-run the noise generator because the chunk is already generated (with the structures) together with the mob spawners. I not really understand what is your mod processing in this specific scenario and why?

I only ask you to check your logic and to optimize your "custom" implementation to make sure it has not such a performance impact.

It's totally fine if a chunk get's loaded to have even 10% or 20% load spike, but a 5% load on a already loaded chunk is something which I don't see in other similar mods.

In the case you are open for a PR, I could try to optimize some of the code for using less conditions and more Java 16 features for the 1.18.1 version.

Otherwise I would appreciate every smaller optimization which lowers this load for loaded chunks. Thanks.

commented

Furthermore you bypass some of the standard forge api's and event's,

The forge structure mob spawn event does the same accessor.getStructureAt call. That call is the only main way to tell if a spot is in a structure and is basically what is going to take up the most time since it has to grab the chunk and check for the structure start. You can clone and open up this project in an ide and follow the structure spawn hook that forge does. In fact, if I optimize this somehow, I would end up being more performant than forge or vanilla lol.

In a loaded chunk with an active player, there is normally no need to re-run the noise generator because the chunk is already generated

The chunk is made yes, but mobs spawn at runtime. You know how when you kill mobs around that more spawn afterwards? What Minecraft does is if the mobs around is below the mob cap, it’ll keep picking random points around you and try to spawn more of those mobs. If the spot ends up in certain structures, it’ll spawn those structure specific mobs. This is why vanilla’s nether fortress spawns wither skeletons instead of the biome’s normal spawns and why witch huts are able to be turned into a mob grinder for witches.

the structure creation is done yes. No more code related to that is ran. The game is now doing mob spawns which is after worldgen and is ran all the time. This is what I hooked into in order to do the same overtime structure spawns like vanilla does in the same way that vanilla is doing it. If you go into vanilla’s NoiseChunkGenerator, you’ll see the same accessor.getStructureAt call to check if the spot is in the structure and spawns that structures mob. My mixin is adding my code that does the exact same thing to the end of that method so I’m actually doing my structure mob spawn code in the same place that vanilla does it to help prevent any conflicts or issues down the road from compat.

The only thing I could really see I can do is create a new method similar to this method I made before that tries to copy and break down the internals of accessor.getStructureAt to do some chunk caching which may help speed it up. Other than that, there isn’t much I can do and switching to Java 16 language features isn’t going to help lol

public static boolean inStructureBounds(WorldGenLevel level, SectionPos refSectionPos, RSStructureTagMap.STRUCTURE_TAGS structureTag) {

commented

And now I'm gonna seem like an ass as I talked with some modder friends about this to be sure and someone pointed out that vanilla does a !p_158434_.hasAnyStructureAt(p_158436_) check to make sure there is a structure at the spot first before doing the structure specific checks. Turns out, it's a new method introduced in 1.18 which is why i missed it in my porting to it oops.

So yeah, two optimizations i can try. Add !p_158434_.hasAnyStructureAt(p_158436_) check and cache the structure specific checks. WIll try this later tonight

commented

Thanks a lot for the update and no worries. You provided a lot of additional details to your answers which I really appreciated.

Glad that you find something. I will try to see if there is a way to optimize structures in general over a additional module for my performance mod in the long term.

Happy modding. ;)

commented

So after more poking around, minecraft will do roughly 2800 mob spawning attempts every second even when standing still. It can go up to even 14000 if flying around. It is just pure random position picking that mc does.

I brought that up as when roughly microbenchmarking the current code I have, running it 100,000,000 times in a loop only takes up 1 second and if i use the hasAnyStructureAt method beforehand, it makes it 2 seconds (probably because it makes a new map inside and section pos). So the mob spawn code I have is already insanely fast (lot faster than I expected).

So instead, what I did was just moved my mixin to inject my structure mob checking to within minecraft's own hasAnyStructureAt check so mc is the one that does it and if it finds any structure, then my structure code will run. But yeah, since it's already super fast, i think it was only showing up on spark because mc mob spawning is just blasting a ton of attempts every second but not noticable in actual gameplay unless you profile it as it accumulates all of those attempts together in a report.

though in v4.2.111 RS out now, mc's check should be showing up instead since I'm just freeloading on it's check now and shaving off a few microseconds every tick or so lol. thank you for the report tho! hopefully that helps

commented

Uhh. I think there’s a misunderstanding here… that method there is basically exactly like how vanilla does it. Every time the game goes to try to spawn a mob, it checks if the spot is within bounds of a structure. For vanilla, it’ll check if it is in bounds of a nether fortress or witch hut or whatever. If so, it then grabs that structure’s mob list and uses one mob from there to spawn.

Repurposed structures does the exact same accessor.getStructureAt check basically so if anything, this is normal behavior of how vanilla works. If another mod can optimize getting structure starts from chunks, that’ll greatly benefit not just my mod but vanilla as well.

if (!structureEntry.getValue().isEmpty() && accessor.getStructureAt(pos, structureEntry.getKey()).isValid()) {

But again, this is only occurring when spawning mobs which means that it will always be showing up at a high percentage in Spark because mob spawning happens 24/7 and the code path is always nearly the same. It’s basically expected.

I can try to see if there anything I could optimize but it is extremely unlikely without getting into very cursed code or blowing something up lol. Or if I can get an optimization, it may not have any measurable impact.

if the mob spawning of the game is still too much, you can turn off the structure based mob spawning from repurposed structures by going into my natural spawning config and remove all the structures from it.