Create

Create

86M Downloads

Fluid handling causing progressively rising CPU usage

DejaVuMan opened this issue ยท 6 comments

commented

Describe the Bug

An idle world with the Create 0.5.0g version of the mod will cause progressively higher CPU usage over the span of a couple hours if multiple hose pulley blocks exist in the world and draw from bottomless fluid sources. This bug was not present earlier in version 0.5.0c for certain, as idle usage after a restart sat at ~20%. Now, idle usage after a restart begins ~20% before rising to >90% utilization of allocated CPU resources.

My specific instance of the issue occurs with 3 hose pulley blocks pulling fluids from bottomless sources with one existing in the Nether as well (Nether has a lava pumping station set up with the chunk set to always loaded utilizing the /forceload command) - I am in the process of testing if this same issue occurs with 1 hose pulley in the overworld, 1 hose pulley in a different dimension (also each utilizing /forceload), without any of the mods in the list other than Create and dependencies.

I am fairly sure this is not a conflict with any other mod as the only differences implemented since 0.5.0c was used previously and the issue didn't occur was an update of Create to 0.5.0g, Create Additions updated to 1.18.2-20221219a, and Flywheel was updated to 0.6.8-97.

I've also included Spark profiling and heap summary:
https://spark.lucko.me/jKk3Sfutyl
https://spark.lucko.me/YI3hRauBSH

Reproduction Steps (Edited on 12/28/2022)

  1. Place a Nether Portal in the overworld, travel to the Nether.
  2. Have a source of fluid which can be considered infinite in the Nether, but is accessible from a chunk not more than 5 chunks away from the Nether portal.
  3. Place a Hose Pulley block pulling fluid resources into a Fluid Tank with a Mechanical Pump.
  4. Use the /forceload command within the chunk where the hose pulley exists
  5. Restart the server.
  6. Compare CPU utilization from the start of server idle time (after world generation, etc) to 6 hours later idle time.
    ...

Expected Result

I would expect CPU utilization to remain at the "start" (Roughly 5 minutes after everything is loaded up) idle time and not continuously rise over time.

Screenshots and Videos

No response

Crash Report or Log

https://gist.github.com/DejaVuMan/49ab822e921ffff5b0d4f73a2404648c | https://gist.github.com/DejaVuMan/4dc476a943783a44fb5ba987705c4fa6

Operating System

Windows 11

Mod Version

0.5.0g

Minecraft Version

1.18.2

Forge Version

40.1.85

Other Mods

  • Alex's Mods (1.18.6)
  • Ambient Sounds (5.0.16)
  • Aquaculture 2 (2.3.7)
  • Bad Packets API (0.1.2)
  • Balm (3.1.0+0)
  • Bigger Reactors (0.6.0-beta.1.2)
  • Citadel (1.11.3)
  • CraftTweaker (9.1.174)
  • Create Crafts & Additions (20221219a)
  • Create Stuff Additions (1.1.8)
  • CreativeCore (2.6.15)
  • Curios API (5.0.7.1)
  • Dungeons Enhanced (3.2.1)
  • Easy Magic (3.3.0)
  • Enhanced Mob Spawners (1.8.17)
  • Epic Knights: Armor and Weapons (1.0)
  • FallingTree (3.5.3)
  • FastSuite (3.0.2)
  • Ferrite Core (4.2.1)
  • Flywheel API (0.6.8-97)
  • Goblin Traders (1.7.2)
  • Immersive Engineering (8.0.2-149)
  • Industrial Foregoing (3.3.1.4)
  • JEI Enchantment Info (2.0.0)
  • JEI Integration (9.0.0.37)
  • JEI Tweaker (3.0.0.8)
  • Jobs+ (0.4.0)
  • Journeymap (5.9.0beta1)
  • Just Enough Items (9.7.1.232)
  • Just Enough Resources (0.14.1.171)
  • Krypton Reforged (0.1.10-SNAPSHOT)
  • Mekanism (10.2.5)
  • Mekanism: Generators (10.2.5)
  • MrCrayfish's Furniture Mod (7.0.0-pre29)
  • Oh The BIomes You'll Go (1.4)
  • Paintings++ (9.1.2.1)
  • Pam's HarvestCraft 2 - Crops+Food Core+Food Extended+Trees (1.0.1)
  • Phospophyllite (0.6.0-beta.1.4)
  • Polymorph (0.44)
  • Puzzles Lib (3.3.5)
  • Quartz (0.0.0-beta.1.3)
  • Radium (0.7.10)
  • Refined Storage (1.10.2)
  • Starlight (1.0.2+forge.83663de)
  • Structure Gel API (2.4.6)
  • TerraBlender (1.1.0.102)
  • Titanium (3.5.6)
  • Vein Mining (0.18)
  • Visual Workbench (3.3.0)
  • Waystones (10.1.0)
  • When Dungeons Arise (2.1.50d)
  • wthit (4.11.0)

Additional Context

Heap Dump: https://www.mediafire.com/file/6slxvk1fwy8h99s/heap-2022-12-27_11.53.39.hprof/file
No response

commented

I'm still in the process of testing this in an isolated environment, but spark heap dump summary and profiling indicate FluidManipulationBehavior.search() seems to be the root cause of this problem doing some operations related to ArrayList.add() and likely related to a Hashmap.foreach() operation - heap dump summary indicates there are over 17 million instances of blockPos and FluidManipulationBehavior operations each

Edit: I've taken some time to look through a heap dump I managed to obtain as well from the server before shutting it down - open first glance it appears to be related to the Nether dimension, as the fluid classname is LavaFluid. According to Eclipse Memory Viewer and the shortest path to accumulation point analysis, frontier from FluidDrainingBehavior has a shallow heap size of 80 and a retained heap of over 326,000,000. Heap dump linked above
Still not sure exactly what could be the cause of this though, keeping the chunk forceloaded shouldn't have this kind of effect and this issue hasn't reared its head in an isolated environment yet where I try to recreate the same scenario (same world except with only 11 mods).

commented

It seems increasingly likely this is due to some mod conflict? I am unable to replicate it on my own personal server. I'm testing each mod individually to see what might be causing this. So far, I've managed to rule out every mod, although more testing should be done with OTBYG/TerraBlender

commented

So, after testing each mod I had previously to see if the issue occurs without it and it disappearing after removing OTBYG (which coincides with dimensions other than the overworld suddenly not working, thus meaning it isn't forceloaded anymore) it seems as if there are a few possibilities as to the cause of this issue:

  1. Oh The Biomes You'll Go and\or TerraBlender somehow conflict with Create - something with the way Create interacts with Fluids and reports block positions? Probably unlikely
  2. There is some issue with Create's logic regarding checking the size of fluid bodies - if a body of fluid being pulled from is sufficiently large? Not sure about this, but it might explain why the issue occurs if we /forceload a Nether chunk with the Hose Pulley and Mechanical Pump active.
  3. This is some issue specific to different Java Versions or runtime environments? (Linux 5.15.0-53-generic vs Windows 11, Java 17.0.2 vs 17.0.3)
  4. Some other issue which I haven't thought of (Maybe removing and replacing the Hose Pulley blocks has some impact, etc)?

EDIT: Removing and replacing the Hose Pulley block solves the issue while the server is up, but as soon as the server is restarted, the issue occurs all over again - I think we can most definitely confirm this is some issue related to keeping the chunk itself loaded while the rest of the dimension isnt necessarily loaded? not too certain, hopefully after the holiday season we can figure out what this odd issue is caused by or how to resolve it.

commented

Ok, so I think I have finally figured out how to reproduce this issue, and I will also update the original reproduction steps at the beginning, but basically:

If you create a Nether portal and go out 3 to 4 chunks from it, place a hose pulley and mechanical pump into a lava source large enough to be considered at least Infinite, pump from it actively, and keep that chunk forceloaded, upon restart of the server the instances of net.minecraft.core.BlockPos and of com.simibubi.create.content.contraptions.fluids.actors.FluidManipulationBehaviour$BlockPosEntry will grow at what seems like a constant rate (as observed here via Spark's Heap summary when filtering for 'blockpos': 38s in, 1m 7s in, 4m 48s in), which will eventually lead to high CPU usage as an increasing number of operations are carried out on ArrayLists - perhaos to do with the behavior within the protected Fluid search method within FluidManipulationBehaviour where a new BlockPosEntry is added to frontier.

commented

I think the problem may stem from the conditional in the search() for loop:

for (i = 0; i < searchedPerTick && !frontier.isEmpty()
&& (visited.size() <= maxBlocks || !canDrainInfinitely(fluid)); i++) {

It is possible for the search to continue adding entries to the visited HashSet beyond the maxBlocks limit (default: 10000) if !canDrainInfinitely(fluid) evaluates to True.

From profiling stack traces that give line numbers, you can see that it's the calls to visited.contains() that all the CPU time is being spent on. When visited gets large enough, it performs poorly.

But checking membership using a hash should be a fixed time regardless of the size, right? Well it seems that the Java HashSet implementation uses a hash function (the same one as HashMap) that suffers from hash collisions if the set gets large enough. Also, the nature of the data being hashed in this case may also be increasing the likelihood of collisions.

The search will consider every connected fluid block within a spherical radius of the hose pulley range (128) and ultimately add them to visited. A maximum of almost 2.8 million blocks, though you'd get less than half in practice, say by placing over a large deep ocean.


There are 2 considerations:

  1. Is the conditional in the for loop correct?
  2. If it is, is there a better way for the search to ignore visited blocks?
commented

This behavior appears to be the same as to what I've experienced TigerWalts - assuming frontier is never empty and like you mentioned canDrainInfinitely evaluates to True, the for loop can ignore the maxBlocks limit.

It seems as if that part can be resolved by splitting the multiple entry condition for loop into a for loop with if statements:

        for (int i = 0; i < searchedPerTick; i++)
        {
            if(frontier.isEmpty())
            {
                break; // exit out of loop - or continue if we want to go to next iteration
            }
            
            if(visited.size() > maxBlocks)
            {
                break;
            }
            
            if(canDrainInfinitely)
            {
                break;
            }
            ... // main logic
        }

That would have the same performance as the current for loop due to how Java compiles it anyway and improve readability as it is much more obvious what conditions need to be met before getting to the main logic portion of the for loop

I believe break would be the correct statement to use as it exists the for loop completely, but continue could be used if we wanted to go to the i+1 iteration of the loop

EDIT: However, it seems as if the current implementation used in the latest version has resolved the issue, as I no longer see BlockPosEntry increasing constantly on restart - I believe the issue can be marked as closed