Skip to content

Commit

Permalink
The epitome of laziness
Browse files Browse the repository at this point in the history 
- Only upload changed page frame descriptors
- In the instancer, track changed contents separately from changed
  validity, so we can separately upload objects and descriptors
  • Loading branch information
@Jozufozu
Jozufozu committed Nov 2, 2024
1 parent fc3e475 commit a9f2018
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 36 deletions.
69 changes: 51 additions & 18 deletions .../src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/IndirectInstancer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,19 @@ public class IndirectInstancer<I extends Instance> extends AbstractInstancer<I>
private final Vector4fc boundingSphere;

private final AtomicReference<InstancePage[]> 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.
Expand Down Expand Up @@ -69,6 +81,7 @@ private I[] instanceArray() {
public final class InstancePage implements InstanceHandleImpl.State<I> {
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<I>[] handles;
/**
* A bitset describing which indices in the instances/handles arrays contain live instances.
Expand Down Expand Up @@ -116,7 +129,8 @@ public boolean add(I instance, InstanceHandleImpl<I> 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.
Expand All @@ -140,7 +154,7 @@ private int local2HandleIndex(int index) {

@Override
public InstanceHandleImpl.State<I> setChanged(int index) {
changedPages.set(pageNo);
contentsChanged.set(pageNo);
return this;
}

Expand All @@ -156,7 +170,7 @@ public InstanceHandleImpl.State<I> 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);
Expand All @@ -182,19 +196,21 @@ public InstanceHandleImpl.State<I> setVisible(InstanceHandleImpl<I> 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;
}
Expand All @@ -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.
Expand All @@ -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);
Expand All @@ -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;
}
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -501,7 +534,7 @@ public int instanceCount() {
*/
public void clear() {
this.pages.set(pageArray(0));
changedPages.clear();
contentsChanged.clear();
fullPages.clear();

}
Expand Down
43 changes: 25 additions & 18 deletions common/src/backend/java/dev/engine_room/flywheel/backend/engine/indirect/ObjectStorage.java
View file
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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.
*/
Expand All @@ -31,8 +33,6 @@ public class ObjectStorage extends AbstractArena {
*/
private MemoryBlock frameDescriptors;

private boolean needsUpload = false;

public ObjectStorage(long objectSizeBytes) {
super(PAGE_SIZE * objectSizeBytes);

Expand Down Expand Up @@ -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
Expand All @@ -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() {
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit a9f2018

Please sign in to comment.