From a9f2018c0a267216bdc30cf8f30e258d23a7b2be Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sat, 2 Nov 2024 14:01:22 -0700 Subject: [PATCH] The epitome of laziness - Only upload changed page frame descriptors - In the instancer, track changed contents separately from changed validity, so we can separately upload objects and descriptors --- .../engine/indirect/IndirectInstancer.java | 69 ++++++++++++++----- .../engine/indirect/ObjectStorage.java | 43 +++++++----- 2 files changed, 76 insertions(+), 36 deletions(-) diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java index b3a032337..d6a73d61e 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java @@ -27,7 +27,19 @@ public class IndirectInstancer extends AbstractInstancer private final Vector4fc boundingSphere; private final AtomicReference pages; - private final AtomicBitSet changedPages = new AtomicBitSet(); + /** + * The set of pages whose count changed and thus need their descriptor re-uploaded. + */ + private final AtomicBitSet validityChanged = new AtomicBitSet(); + /** + * The set of pages whose content changed and thus need their instances re-uploaded. + * Note that we don't re-upload for deletions, as the memory becomes invalid and masked out by the validity bits. + */ + private final AtomicBitSet contentsChanged = new AtomicBitSet(); + /** + * The set of pages that are entirely full. + * We scan the clear bits of this set when trying to add an instance. + */ private final AtomicBitSet fullPages = new AtomicBitSet(); /** * The set of mergable pages. A page is mergeable if it is not empty and has 16 or fewer instances. @@ -69,6 +81,7 @@ private I[] instanceArray() { public final class InstancePage implements InstanceHandleImpl.State { private final int pageNo; private final I[] instances; + // Handles are only read in #takeFrom. It would be nice to avoid tracking these at all. private final InstanceHandleImpl[] handles; /** * A bitset describing which indices in the instances/handles arrays contain live instances. @@ -116,7 +129,8 @@ public boolean add(I instance, InstanceHandleImpl handle) { // Handle index is unique amongst all pages of this instancer. handle.index = local2HandleIndex(index); - changedPages.set(pageNo); + contentsChanged.set(pageNo); + validityChanged.set(pageNo); if (isFull(newValue)) { // The page is now full, mark it so in the bitset. // This is safe because only one bit position changes at a time. @@ -140,7 +154,7 @@ private int local2HandleIndex(int index) { @Override public InstanceHandleImpl.State setChanged(int index) { - changedPages.set(pageNo); + contentsChanged.set(pageNo); return this; } @@ -156,7 +170,7 @@ public InstanceHandleImpl.State setDeleted(int index) { int newValue = currentValue & ~(1 << localIndex); if (valid.compareAndSet(currentValue, newValue)) { - changedPages.set(pageNo); + validityChanged.set(pageNo); if (isEmpty(newValue)) { // If we decremented to zero then we're no longer mergeable. mergeablePages.clear(pageNo); @@ -182,19 +196,21 @@ public InstanceHandleImpl.State setVisible(InstanceHandleImpl handle, int return new InstanceHandleImpl.Hidden<>(recreate, instances[localIndex]); } - public void takeFrom(InstancePage other) { + /** + * Only call this on 2 pages that are mergeable. + * + * @param other The page to take instances from. + */ + private void takeFrom(InstancePage other) { // Fill the holes in this page with instances from the other page. int valid = this.valid.get(); int otherValid = other.valid.get(); - // Something is going to change, so mark stuff ahead of time. - changedPages.set(pageNo); - changedPages.set(other.pageNo); - for (int i = 0; i < ObjectStorage.PAGE_SIZE; i++) { int mask = 1 << i; + // Find set bits in the other page. if ((otherValid & mask) == 0) { continue; } @@ -212,7 +228,7 @@ public void takeFrom(InstancePage other) { other.handles[i] = null; other.instances[i] = null; - // Set the bit in this page and find the next write position. + // Set the bit in this page so we can find the next write position. valid |= 1 << writePos; // If we're full, we're done. @@ -224,9 +240,18 @@ public void takeFrom(InstancePage other) { this.valid.set(valid); other.valid.set(otherValid); + // If the other page was quite empty we may still be mergeable. mergeablePages.set(pageNo, isMergeable(valid)); - // With all assumptions in place we should have fully drained the other page, but check just to be safe. - mergeablePages.set(other.pageNo, isMergeable(otherValid)); + + // We definitely changed the contents and validity of this page. + contentsChanged.set(pageNo); + validityChanged.set(pageNo); + + // The other page will end up empty, so the validity changes and it's no longer mergeable. + // Also clear the changed bit so we don't re-upload the instances. + contentsChanged.clear(other.pageNo); + validityChanged.set(other.pageNo); + mergeablePages.clear(other.pageNo); if (isFull(valid)) { fullPages.set(pageNo); @@ -246,7 +271,7 @@ public void update(int modelIndex, int baseInstance) { this.baseInstance = baseInstance; var sameModelIndex = this.modelIndex == modelIndex; - if (sameModelIndex && changedPages.isEmpty()) { + if (sameModelIndex && validityChanged.isEmpty()) { // Nothing to do! return; } @@ -258,7 +283,7 @@ public void update(int modelIndex, int baseInstance) { if (sameModelIndex) { // Only need to update the changed pages. - for (int page = changedPages.nextSetBit(0); page >= 0 && page < pages.length; page = changedPages.nextSetBit(page + 1)) { + for (int page = validityChanged.nextSetBit(0); page >= 0 && page < pages.length; page = validityChanged.nextSetBit(page + 1)) { mapping.updatePage(page, modelIndex, pages[page].valid.get()); } } else { @@ -267,6 +292,8 @@ public void update(int modelIndex, int baseInstance) { mapping.updatePage(i, modelIndex, pages[i].valid.get()); } } + + validityChanged.clear(); } public void writeModel(long ptr) { @@ -280,15 +307,21 @@ public void writeModel(long ptr) { } public void uploadInstances(StagingBuffer stagingBuffer, int instanceVbo) { - if (changedPages.isEmpty()) { + if (contentsChanged.isEmpty()) { return; } var pages = this.pages.get(); - for (int page = changedPages.nextSetBit(0); page >= 0 && page < pages.length; page = changedPages.nextSetBit(page + 1)) { + for (int page = contentsChanged.nextSetBit(0); page >= 0 && page < pages.length; page = contentsChanged.nextSetBit(page + 1)) { var instances = pages[page].instances; long baseByte = mapping.page2ByteOffset(page); + + if (baseByte < 0) { + // This page is not mapped to the VBO. + continue; + } + long size = ObjectStorage.PAGE_SIZE * instanceStride; // Because writes are broken into pages, we end up with significantly more calls into @@ -321,7 +354,7 @@ public void uploadInstances(StagingBuffer stagingBuffer, int instanceVbo) { stagingBuffer.enqueueCopy(block.ptr(), size, instanceVbo, baseByte); } - changedPages.clear(); + contentsChanged.clear(); } public void parallelUpdate() { @@ -501,7 +534,7 @@ public int instanceCount() { */ public void clear() { this.pages.set(pageArray(0)); - changedPages.clear(); + contentsChanged.clear(); fullPages.clear(); } diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java index 20b6ee87a..d7c7e67b3 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java @@ -1,6 +1,7 @@ package dev.engine_room.flywheel.backend.engine.indirect; import java.util.Arrays; +import java.util.BitSet; import org.lwjgl.system.MemoryUtil; @@ -18,6 +19,7 @@ public class ObjectStorage extends AbstractArena { public static final int INITIAL_PAGES_ALLOCATED = 4; public static final int DESCRIPTOR_SIZE_BYTES = Integer.BYTES * 2; + private final BitSet changedFrames = new BitSet(); /** * The GPU side buffer containing all the objects, logically divided into page frames. */ @@ -31,8 +33,6 @@ public class ObjectStorage extends AbstractArena { */ private MemoryBlock frameDescriptors; - private boolean needsUpload = false; - public ObjectStorage(long objectSizeBytes) { super(PAGE_SIZE * objectSizeBytes); @@ -62,6 +62,16 @@ public void free(int i) { var ptr = ptrForPage(i); MemoryUtil.memPutInt(ptr, 0); MemoryUtil.memPutInt(ptr + 4, 0); + + changedFrames.set(i); + } + + private void set(int i, int modelIndex, int validBits) { + var ptr = ptrForPage(i); + MemoryUtil.memPutInt(ptr, modelIndex); + MemoryUtil.memPutInt(ptr + 4, validBits); + + changedFrames.set(i); } @Override @@ -72,12 +82,17 @@ protected void grow() { } public void uploadDescriptors(StagingBuffer stagingBuffer) { - if (!needsUpload) { + if (changedFrames.isEmpty()) { return; } - // We could be smarter about which spans are uploaded but this thing is so small it's probably not worth it. - stagingBuffer.enqueueCopy(frameDescriptors.ptr(), frameDescriptors.size(), frameDescriptorBuffer.handle(), 0); - needsUpload = false; + + var ptr = frameDescriptors.ptr(); + for (int i = changedFrames.nextSetBit(0); i >= 0 && i < capacity(); i = changedFrames.nextSetBit(i + 1)) { + var offset = (long) i * DESCRIPTOR_SIZE_BYTES; + stagingBuffer.enqueueCopy(ptr + offset, DESCRIPTOR_SIZE_BYTES, frameDescriptorBuffer.handle(), offset); + } + + changedFrames.clear(); } public void delete() { @@ -110,18 +125,14 @@ public void updatePage(int index, int modelIndex, int validBits) { holePunch(index); return; } - var page = pages[index]; + var frame = pages[index]; - if (page == INVALID_PAGE) { + if (frame == INVALID_PAGE) { // Un-holed punch. - page = unHolePunch(index); + frame = unHolePunch(index); } - var ptr = ptrForPage(page); - MemoryUtil.memPutInt(ptr, modelIndex); - MemoryUtil.memPutInt(ptr + 4, validBits); - - ObjectStorage.this.needsUpload = true; + ObjectStorage.this.set(frame, modelIndex, validBits); } /** @@ -132,8 +143,6 @@ public void updatePage(int index, int modelIndex, int validBits) { public void holePunch(int index) { ObjectStorage.this.free(pages[index]); pages[index] = INVALID_PAGE; - - ObjectStorage.this.needsUpload = true; } /** @@ -173,8 +182,6 @@ public void delete() { ObjectStorage.this.free(page); } pages = EMPTY_ALLOCATION; - - ObjectStorage.this.needsUpload = true; } private void grow(int neededPages, int oldLength) {