Section tracker core rewrite

src/main/java/me/cortex/voxelmon/core/rendering/RenderTracker.java

@@ -54,7 +54,7 @@ public class RenderTracker {
                             continue;
 
                         for (int y = -3>>i; y < Math.max(1, 10 >> i); y++) {
-                            var sec = this.world.getOrLoadAcquire(i, x + (OX>>(1+i)), y, z + (OZ>>(1+i)));
+                            var sec = this.world.acquire(i, x + (OX>>(1+i)), y, z + (OZ>>(1+i)));
                             //this.renderGen.enqueueTask(sec);
                             sec.release();
                         }

src/main/java/me/cortex/voxelmon/core/rendering/building/RenderDataFactory.java

@@ -38,9 +38,11 @@ public class RenderDataFactory {
         // appearing between lods
 
 
-        if (section.definitelyEmpty()) {
+        //if (section.definitelyEmpty()) {//Fast path if its known the entire chunk is empty
-            return new BuiltSectionGeometry(section.getKey(), null, null);
+        //    return new BuiltSectionGeometry(section.getKey(), null, null);
-        }
+        //}
+
+
         var data = section.copyData();
 
         long[] connectedData = null;
@@ -64,7 +66,7 @@ public class RenderDataFactory {
                         if (y == 31 && ((buildMask>>(6+dirId))&1) == 0) {
                             //Load and copy the data into a local cache, TODO: optimize so its not doing billion of copies
                             if (connectedData == null) {
-                                var connectedSection = this.world.getOrLoadAcquire(section.lvl, section.x, section.y + 1, section.z);
+                                var connectedSection = this.world.acquire(section.lvl, section.x, section.y + 1, section.z);
                                 connectedData = connectedSection.copyData();
                                 connectedSection.release();
                             }
@@ -106,7 +108,7 @@ public class RenderDataFactory {
                         if (x == 31 && ((buildMask>>(6+dirId))&1) == 0) {
                             //Load and copy the data into a local cache, TODO: optimize so its not doing billion of copies
                             if (connectedData == null) {
-                                var connectedSection = this.world.getOrLoadAcquire(section.lvl, section.x + 1, section.y, section.z);
+                                var connectedSection = this.world.acquire(section.lvl, section.x + 1, section.y, section.z);
                                 connectedData = connectedSection.copyData();
                                 connectedSection.release();
                             }
@@ -148,7 +150,7 @@ public class RenderDataFactory {
                         if (z == 31 && ((buildMask>>(6+dirId))&1) == 0) {
                             //Load and copy the data into a local cache, TODO: optimize so its not doing billion of copies
                             if (connectedData == null) {
-                                var connectedSection = this.world.getOrLoadAcquire(section.lvl, section.x, section.y, section.z + 1);
+                                var connectedSection = this.world.acquire(section.lvl, section.x, section.y, section.z + 1);
                                 connectedData = connectedSection.copyData();
                                 connectedSection.release();
                             }
@@ -190,7 +192,7 @@ public class RenderDataFactory {
                         if (x == 0 && ((buildMask>>(6+dirId))&1) == 0) {
                             //Load and copy the data into a local cache, TODO: optimize so its not doing billion of copies
                             if (connectedData == null) {
-                                var connectedSection = this.world.getOrLoadAcquire(section.lvl, section.x - 1, section.y, section.z);
+                                var connectedSection = this.world.acquire(section.lvl, section.x - 1, section.y, section.z);
                                 connectedData = connectedSection.copyData();
                                 connectedSection.release();
                             }
@@ -232,7 +234,7 @@ public class RenderDataFactory {
                         if (z == 0 && ((buildMask>>(6+dirId))&1) == 0) {
                             //Load and copy the data into a local cache, TODO: optimize so its not doing billion of copies
                             if (connectedData == null) {
-                                var connectedSection = this.world.getOrLoadAcquire(section.lvl, section.x, section.y, section.z - 1);
+                                var connectedSection = this.world.acquire(section.lvl, section.x, section.y, section.z - 1);
                                 connectedData = connectedSection.copyData();
                                 connectedSection.release();
                             }
@@ -274,7 +276,7 @@ public class RenderDataFactory {
                         if (y == 0 && ((buildMask>>(6+dirId))&1) == 0) {
                             //Load and copy the data into a local cache, TODO: optimize so its not doing billion of copies
                             if (connectedData == null) {
-                                var connectedSection = this.world.getOrLoadAcquire(section.lvl, section.x, section.y - 1, section.z);
+                                var connectedSection = this.world.acquire(section.lvl, section.x, section.y - 1, section.z);
                                 connectedData = connectedSection.copyData();
                                 connectedSection.release();
                             }

src/main/java/me/cortex/voxelmon/core/rendering/building/RenderGenerationService.java

@@ -81,7 +81,7 @@ public class RenderGenerationService {
     public void enqueueTask(int lvl, int x, int y, int z) {
         this.taskQueue.add(new BuildTask(()->{
             if (this.tracker.shouldStillBuild(lvl, x, y, z)) {
-                return this.world.getOrLoadAcquire(lvl, x, y, z);
+                return this.world.acquire(lvl, x, y, z);
             } else {
                 return null;
             }

src/main/java/me/cortex/voxelmon/core/util/VolatileHolder.java

@@ -0,0 +1,5 @@
+package me.cortex.voxelmon.core.util;
+
+public class VolatileHolder <T> {
+    public T obj;
+}

src/main/java/me/cortex/voxelmon/core/world/ActiveSectionTracker.java

@@ -0,0 +1,99 @@
+package me.cortex.voxelmon.core.world;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
+import me.cortex.voxelmon.core.util.VolatileHolder;
+
+import java.lang.ref.Reference;
+
+public class ActiveSectionTracker {
+    //Deserialize into the supplied section, returns true on success, false on failure
+    public interface SectionLoader {boolean load(WorldSection section);}
+
+    //Loaded section world cache, TODO: get rid of VolatileHolder and use something more sane
+
+    private final Long2ObjectOpenHashMap<VolatileHolder<WorldSection>>[] loadedSectionCache;
+    private final SectionLoader loader;
+    public ActiveSectionTracker(int layers, SectionLoader loader) {
+        this.loader = loader;
+        this.loadedSectionCache = new Long2ObjectOpenHashMap[layers];
+        for (int i = 0; i < layers; i++) {
+            this.loadedSectionCache[i] = new Long2ObjectOpenHashMap<>(1<<(16-i));
+        }
+    }
+
+    public WorldSection acquire(int lvl, int x, int y, int z) {
+        long key = WorldEngine.getWorldSectionId(lvl, x, y, z);
+        var cache = this.loadedSectionCache[lvl];
+        VolatileHolder<WorldSection> holder = null;
+        boolean isLoader = false;
+        synchronized (cache) {
+            holder = cache.get(key);
+            if (holder == null) {
+                holder = new VolatileHolder<>();
+                cache.put(key, holder);
+                isLoader = true;
+            }
+            var section = holder.obj;
+            if (section != null) {
+                section.acquire();
+                return section;
+            }
+        }
+        //If this thread was the one to create the reference then its the thread to load the section
+        if (isLoader) {
+            var section = new WorldSection(lvl, x, y, z, this);
+            if (!this.loader.load(section)) {
+                //TODO: Instead if throwing an exception do something better
+                throw new IllegalStateException("Unable to load section");
+            }
+            section.acquire();
+            holder.obj = section;
+            return section;
+        } else {
+            WorldSection section = null;
+            while ((section = holder.obj) == null)
+                Thread.onSpinWait();
+
+            synchronized (cache) {
+                if (section.tryAcquire()) {
+                    return section;
+                }
+            }
+            return this.acquire(lvl, x, y, z);
+        }
+    }
+
+    void tryUnload(WorldSection section) {
+        var cache = this.loadedSectionCache[section.lvl];
+        synchronized (cache) {
+            if (section.trySetFreed()) {
+                if (cache.remove(section.getKey()).obj != section) {
+                    throw new IllegalStateException("Removed section not the same as the referenced section in the cache");
+                }
+            }
+        }
+    }
+
+
+    public int[] getCacheCounts() {
+        int[] res = new int[this.loadedSectionCache.length];
+        for (int i = 0; i < this.loadedSectionCache.length; i++) {
+            res[i] = this.loadedSectionCache[i].size();
+        }
+        return res;
+    }
+
+
+    public static void main(String[] args) {
+        var tracker = new ActiveSectionTracker(1, a->true);
+
+        var section = tracker.acquire(0,0,0,0);
+        section.acquire();
+        var section2 = tracker.acquire(0,0,0,0);
+        section.release();
+        section.release();
+        section = tracker.acquire(0,0,0,0);
+        section.release();
+
+    }
+}

src/main/java/me/cortex/voxelmon/core/world/SaveLoadSystem.java

@@ -68,7 +68,7 @@ public class SaveLoadSystem {
         return out;
     }
 
-    public static WorldSection deserialize(WorldEngine world, int lvl, int x, int y, int z, byte[] data) {
+    public static boolean deserialize(WorldSection section, byte[] data) {
         var buff = MemoryUtil.memAlloc(data.length);
         buff.put(data);
         buff.rewind();
@@ -89,8 +89,6 @@ public class SaveLoadSystem {
             hash ^= lut[i];
         }
 
-        var section = new WorldSection(lvl, x, y, z, world);
-        section.definitelyEmpty = false;
         if (section.getKey() != key) {
             throw new IllegalStateException("Decompressed section not the same as requested. got: " + key + " expected: " + section.getKey());
         }
@@ -107,16 +105,16 @@ public class SaveLoadSystem {
         if (expectedHash != hash) {
             //throw new IllegalStateException("Hash mismatch got: " + hash + " expected: " + expectedHash);
             System.err.println("Hash mismatch got: " + hash + " expected: " + expectedHash + " removing region");
-            return null;
+            return false;
         }
 
         if (decompressed.hasRemaining()) {
             //throw new IllegalStateException("Decompressed section had excess data");
             System.err.println("Decompressed section had excess data removing region");
-            return null;
+            return false;
         }
         MemoryUtil.memFree(decompressed);
 
-        return section;
+        return true;
     }
 }

src/main/java/me/cortex/voxelmon/core/world/WorldEngine.java

@@ -9,6 +9,7 @@ import me.cortex.voxelmon.core.world.service.VoxelIngestService;
 import me.cortex.voxelmon.core.world.storage.StorageBackend;
 
 import java.io.File;
+import java.util.Arrays;
 import java.util.Deque;
 import java.util.concurrent.ConcurrentLinkedDeque;
 import java.util.concurrent.atomic.AtomicReference;
@@ -20,6 +21,7 @@ public class WorldEngine {
 
     public final StorageBackend storage;
     private final Mapper mapper;
+    private final ActiveSectionTracker sectionTracker;
     public final VoxelIngestService ingestService = new VoxelIngestService(this);
     public final SectionSavingService savingService;
     private RenderTracker renderTracker;
@@ -32,152 +34,40 @@ public class WorldEngine {
 
     private final int maxMipLevels;
 
-    //Loaded section world cache
-    private final Long2ObjectOpenHashMap<WorldSection>[] loadedSectionCache;
-    //TODO: also segment this up into an array
-    private final Long2ObjectOpenHashMap<AtomicReference<WorldSection>> sectionLoadingLocks = new Long2ObjectOpenHashMap<>();
-
-    //What this is used for is to keep N sections acquired per layer, this stops sections from constantly being
-    // loaded and unloaded when accessed close together
-    private final ConcurrentLinkedDeque<WorldSection>[] activeSectionCache;
-
 
     public WorldEngine(File storagePath, int savingServiceWorkers, int maxMipLayers) {
         this.maxMipLevels = maxMipLayers;
-        this.loadedSectionCache = new Long2ObjectOpenHashMap[maxMipLayers];
-        this.activeSectionCache = new ConcurrentLinkedDeque[maxMipLayers];
-        for (int i = 0; i < maxMipLayers; i++) {
-            this.loadedSectionCache[i] = new Long2ObjectOpenHashMap<>(1<<(16-i));
-            this.activeSectionCache[i] = new ConcurrentLinkedDeque<>();
-        }
         this.storage = new StorageBackend(storagePath);
         this.mapper = new Mapper(this.storage);
+        this.sectionTracker = new ActiveSectionTracker(maxMipLayers, this::unsafeLoadSection);
+
         this.savingService = new SectionSavingService(this, savingServiceWorkers);
     }
 
+    private boolean unsafeLoadSection(WorldSection into) {
+        var data = this.storage.getSectionData(into.getKey());
+        if (data != null) {
+            if (!SaveLoadSystem.deserialize(into, data)) {
+                this.storage.deleteSectionData(into.getKey());
+                //TODO: regenerate the section from children
+                Arrays.fill(into.data, Mapper.AIR);
+                System.err.println("Section " + into.lvl + ", " + into.x + ", " + into.y + ", " + into.z + " was unable to load, setting to air");
+                return true;
+            }
+        }
+        return true;
+    }
+
+    public WorldSection acquire(int lvl, int x, int y, int z) {
+        return this.sectionTracker.acquire(lvl, x, y, z);
+    }
+
     //TODO: Fixme/optimize, cause as the lvl gets higher, the size of x,y,z gets smaller so i can dynamically compact the format
     // depending on the lvl, which should optimize colisions and whatnot
     public static long getWorldSectionId(int lvl, int x, int y, int z) {
         return ((long)lvl<<60)|((long)(y&0xFF)<<52)|((long)(z&((1<<24)-1))<<28)|((long)(x&((1<<24)-1))<<4);//NOTE: 4 bits spare for whatever
     }
 
-    public static int getLvl(long packed) {
-        return (int) (packed>>>60);
-    }
-    public static int getX(long packed) {
-        return (int) ((packed<<12)>>40);
-    }
-    public static int getY(long packed) {
-        return (int) ((packed<<4)>>56);
-    }
-    public static int getZ(long packed) {
-        return (int) ((packed<<4)>>40);
-    }
-
-    //Try to unload the section from the world atomically, this is called from the saving service, or any release call which results in the refcount being 0
-    public void tryUnload(WorldSection section) {
-        synchronized (this.loadedSectionCache[section.lvl]) {
-            if (section.getRefCount() != 0) {
-                section.assertNotFree();
-                return;
-            }
-
-            //TODO: make a thing where it checks if the section is dirty, if it is, enqueue it for a save first and return
-
-            section.setFreed();//TODO: FIXME THIS IS SOMEHOW FAILING
-            var removedSection = this.loadedSectionCache[section.lvl].remove(section.getKey());
-            if (removedSection != section) {
-                throw new IllegalStateException("Removed section not the same as attempted to remove");
-            }
-            if (section.isAcquired()) {
-                throw new IllegalStateException("Section that was just removed got reacquired");
-            }
-        }
-    }
-
-    //Internal helper method for getOrLoad to segment up code
-    private WorldSection unsafeLoadSection(long key, int lvl, int x, int y, int z) {
-        var data = this.storage.getSectionData(key);
-        if (data == null) {
-            return new WorldSection(lvl, x, y, z, this);
-        } else {
-            var ret =  SaveLoadSystem.deserialize(this, lvl, x, y, z, data);
-            if (ret != null) {
-                return ret;
-            } else {
-                this.storage.deleteSectionData(key);
-                return new WorldSection(lvl, x, y, z, this);
-            }
-        }
-    }
-
-    //Gets a loaded section or loads the section from storage
-    public WorldSection getOrLoadAcquire(int lvl, int x, int y, int z) {
-        long key = getWorldSectionId(lvl, x, y, z);
-
-        AtomicReference<WorldSection> lock = null;
-        AtomicReference<WorldSection> gotLock = null;
-        synchronized (this.loadedSectionCache[lvl]) {
-            var result = this.loadedSectionCache[lvl].get(key);
-            if (result != null) {
-                result.acquire();
-                return result;
-            }
-            lock = new AtomicReference<>(null);
-            synchronized (this.sectionLoadingLocks) {
-                var finalLock = lock;
-                gotLock = this.sectionLoadingLocks.computeIfAbsent(key, a -> finalLock);
-            }
-        }
-
-        //We acquired the lock so load it
-        if (gotLock == lock) {
-            WorldSection loadedSection = this.unsafeLoadSection(key, lvl, x, y, z);
-            loadedSection.acquire();
-
-
-            //Insert the loaded section and set the loading lock to the loaded value
-            synchronized (this.loadedSectionCache[lvl]) {
-                this.loadedSectionCache[lvl].put(key, loadedSection);
-                synchronized (this.sectionLoadingLocks) {
-                    this.sectionLoadingLocks.remove(key);
-                    lock.set(loadedSection);
-                }
-            }
-
-            //Add to the active acquired cache and remove the last item if the size is over the limit
-            {
-                loadedSection.acquire();
-                this.activeSectionCache[lvl].add(loadedSection);
-                if (this.activeSectionCache[lvl].size() > ACTIVE_CACHE_SIZE) {
-                    var last = this.activeSectionCache[lvl].pop();
-                    last.release();
-                }
-            }
-
-            return loadedSection;
-        } else {
-            lock = gotLock;
-            //Another thread got the lock so spin wait for the section to load
-            while (lock.get() == null) {
-                Thread.onSpinWait();
-            }
-            var section = lock.get();
-            //Fixme: try find a better solution for this
-
-            //The issue with this is that the section could be unloaded when we acquire it cause of so many threading pain
-            // so lock the section cache, try acquire the section, if we fail we must load the section again
-            synchronized (this.loadedSectionCache[lvl]) {
-                if (section.tryAcquire()) {
-                    //We acquired the section successfully, return it
-                    return section;
-                }
-            }
-            //We failed to acquire the section, we must reload it
-            return this.getOrLoadAcquire(lvl, x, y, z);
-        }
-    }
-
     //Marks a section as dirty, enqueuing it for saving and or render data rebuilding
     private void markDirty(WorldSection section) {
         this.renderTracker.sectionUpdated(section);
@@ -191,7 +81,7 @@ public class WorldEngine {
     public void insertUpdate(VoxelizedSection section) {
         //The >>1 is cause the world sections size is 32x32x32 vs the 16x16x16 of the voxelized section
         for (int lvl = 0; lvl < this.maxMipLevels; lvl++) {
-            var worldSection = this.getOrLoadAcquire(lvl, section.x>>(lvl+1), section.y>>(lvl+1), section.z>>(lvl+1));
+            var worldSection = this.acquire(lvl, section.x >> (lvl + 1), section.y >> (lvl + 1), section.z >> (lvl + 1));
             int msk = (1<<(lvl+1))-1;
             int bx = (section.x&msk)<<(4-lvl);
             int by = (section.y&msk)<<(4-lvl);
@@ -221,11 +111,7 @@ public class WorldEngine {
     }
 
     public int[] getLoadedSectionCacheSizes() {
-        var res = new int[this.maxMipLevels];
+        return this.sectionTracker.getCacheCounts();
-        for (int i = 0; i < this.maxMipLevels; i++) {
-            res[i] = this.loadedSectionCache[i].size();
-        }
-        return res;
     }
 
     public void shutdown() {

src/main/java/me/cortex/voxelmon/core/world/WorldSection.java

@@ -13,20 +13,21 @@ public final class WorldSection {
     public final int y;
     public final int z;
 
-    ////Maps from a local id to global meaning it should be much cheaper to store in memory probably
+    long[] data;
-    //private final int[] dataMapping = null;
+    private final ActiveSectionTracker tracker;
-    //private final short[] data = new short[32*32*32];
+    public final AtomicBoolean inSaveQueue = new AtomicBoolean();
-    final long[] data = new long[32*32*32];
-    boolean definitelyEmpty = true;
 
-    private final WorldEngine world;
+    //When the first bit is set it means its loaded
+    private final AtomicInteger atomicState = new AtomicInteger(1);
 
-    public WorldSection(int lvl, int x, int y, int z, WorldEngine worldIn) {
+    WorldSection(int lvl, int x, int y, int z, ActiveSectionTracker tracker) {
         this.lvl = lvl;
         this.x = x;
         this.y = y;
         this.z = z;
-        this.world = worldIn;
+        this.tracker = tracker;
+
+        this.data = new long[32*32*32];
     }
 
     @Override
@@ -34,65 +35,49 @@ public final class WorldSection {
         return ((x*1235641+y)*8127451+z)*918267913+lvl;
     }
 
-    public final AtomicBoolean inSaveQueue = new AtomicBoolean();
-    private final AtomicInteger usageCounts = new AtomicInteger();
 
     public int acquire() {
-        this.assertNotFree();
+        int state = this.atomicState.addAndGet(2);
-        return this.usageCounts.getAndAdd(1);
+        if ((state&1) == 0) {
+            throw new IllegalStateException("Tried to acquire unloaded section");
         }
-
+        return state>>1;
-    //TODO: Fixme i dont think this is fully thread safe/correct
-    public boolean tryAcquire() {
-        if (this.freed) {
-            return false;
-        }
-        this.usageCounts.getAndAdd(1);
-        if (this.freed) {
-            return false;
-        }
-        return true;
     }
 
     public int release() {
-        this.assertNotFree();
+        int state = this.atomicState.addAndGet(-2);
-        int i = this.usageCounts.addAndGet(-1);
+        if (state < 1) {
-        if (i < 0) {
+            throw new IllegalStateException("Section got into an invalid state");
-            throw new IllegalStateException();
+        }
+        if ((state&1)==0) {
+            throw new IllegalStateException("Tried releasing a freed section");
+        }
+        if ((state>>1)==0) {
+            this.tracker.tryUnload(this);
         }
 
-
+        return state>>1;
-        //NOTE: cant actually check for not free as at this stage it technically could be unloaded, as soon
-        //this.assertNotFree();
-
-
-        //Try to unload the section if its empty
-        if (i == 0) {
-            this.world.tryUnload(this);
-        }
-        return i;
     }
 
-    private volatile boolean freed = false;
+    //Returns true on success, false on failure
-    void setFreed() {
+    boolean trySetFreed() {
-        this.assertNotFree();
+        int witness = this.atomicState.compareAndExchange(1, 0);
-        this.freed = true;
+        if ((witness&1)==0 && witness != 0) {
+            throw new IllegalStateException("Section marked as free but has refs");
+        }
+        boolean isFreed = witness == 1;
+        if (isFreed) {
+            this.data = null;
+        }
+        return isFreed;
     }
 
     public void assertNotFree() {
-        if (this.freed) {
+        if ((this.atomicState.get() & 1) == 0) {
             throw new IllegalStateException();
         }
     }
 
-    public boolean isAcquired() {
-        return this.usageCounts.get() != 0;
-    }
-
-    public int getRefCount() {
-        return this.usageCounts.get();
-    }
-
     public long getKey() {
         return WorldEngine.getWorldSectionId(this.lvl, this.x, this.y, this.z);
     }
@@ -114,11 +99,18 @@ public final class WorldSection {
 
     //Generates a copy of the data array, this is to help with atomic operations like rendering
     public long[] copyData() {
+        this.assertNotFree();
         return Arrays.copyOf(this.data, this.data.length);
     }
 
-    public boolean definitelyEmpty() {
+    public boolean tryAcquire() {
-        return this.definitelyEmpty;
+        int state = this.atomicState.updateAndGet(val -> {
+            if ((val&1) != 0) {
+                return val+2;
+            }
+            return val;
+        });
+        return (state&1) != 0;
     }
 }