World Load Crash w/ Dynamic Surroundings
EpiclyDelicious opened this issue · 13 comments
Loading a new or existing world with this setup causes a crash:
Minecraft Version: 1.10.2
Forge Version: 2202
Foamfix Version: 0.5.0 Anarchy
Dynamic Surroundings Version: 3.2.1.0
Crash Log: https://hastebin.com/tasobufiho.pas
Worked with previous version of the mod (3.2.0.0), so not sure if it is on your end or theirs.
They're not sure what they can do on their end.
DS' Github: OreCruncher/DynamicSurroundings#24
I do have a class that derives from BlockPos.MutableBlockPos and I suspect that is where the problem lies.
Yes, for some reason my coremod is not catching that.
Disable "optimizedBlockPos" in "coremod" as a workaround for the time being.
@asiekierka Sure. The reason I did this is because BlockPos.MutableBlockPos is only partially mutable. :\ I have scanning routines that process 1Ks of blocks per tick and I would have been creating new BlockPos objects very frequently. MutableBlockPos should be fixed up and made final...
EDIT: You know, thinking on this I may have a different way of doing what I need with the position...
@OreCruncher MutableBlockPos has a mutable set method called "setPos". The only thing I'm recommending you to do is to not use @OverRide on the mutable variants of methods (Mojang doesn't either), but implementing separately named mutable variants. First of all, the overrides stop the JIT from doing inlining optimizations; second, overriding methods in this way might break mods which expect said methods to be immutable.
@OreCruncher Also, you might want to consider caching surroundings data and listening to block neighbor changes using something like an IWorldEventListener to greatly improve performance.
@asiekierka The setPos() - that is the change I was looking at. Testing a version of the mod that eliminates my class entirely. As for the caching, is there something specific you have seen?
@OreCruncher No, I just remember how we had it implemented over at BuildCraft, where robots had to be able to quickly query massive areas of the world. Most of the comparisons/checks were cached and updated when necessary.
Gottcha. One of the scanners is a cuboid scanner, and it will only scan the new blocks that enter into range as the player moves, or if there is a notify that comes in and that block is within the cuboid. The other scanner randomly selects blocks over an area around the player, and it uses cached values from the previous block check if possible. I do plan on putting FoamFix in my test environment later today because I am curious as to how it would further improve things.
@OreCruncher Overriding non-mutable BlockPos methods in this way is not a good idea for performance; if you do so, the JIT may have difficulties inlining the methods (which is a good thing to have for methods called thousands of times) - in fact, this is what "optimizedBlockPos" does for the getters. It's a small thing, but still worth considering.
Anyhow, @OreCruncher , @MafooHamhead Could you please try http://asie.pl/foamfix-0.5.1-test1-anarchy.jar ?
@OreCruncher What I'm wondering, however, is: does the build I linked fix the crash?
@asiekierka NVW - found a link to a test JAR above. It passes the sniff test and it appears to be working.