Skip to content

Commit 8707b6b

Browse files
authored
perf(celeritas): sync upstream to f15085d4 (update dispatch + task submission fixes) (#105)
Port upstream celeritas commits b4f69b8f and f15085d4 (cdafa232..f15085d4). b4f69b8f "Fix important updates not taking effect sometimes": - ChunkRebuildLists: the shared static EMPTY record exposed a mutable EnumMap that one pass's rebuild list writes could contaminate with; replaced by a fresh empty() factory per use (RenderListManager). - RenderSectionManager.promoteInterimRebuildList: no longer skipped when a graph update is pending; sections with important update types are still promoted in that case, and the request set is cleared by the method itself. Null pending updates (defensive) are skipped. - scheduleTranslucencyUpdates: inject through rebuildLists.get(update) instead of duplicating the SORT/IMPORTANT_SORT selection. - scheduleSectionForRebuild: decide promotion by the section's new update type (requestUpdate may promote REBUILD to IMPORTANT_REBUILD) instead of the originally requested importance. f15085d4 "Improve robustness of task submission": - ChunkJobTyped.execute: cancelled jobs now still reach their consumer with a null result instead of returning early, so waiting collectors always get their semaphore permit back; started flag is only set for non-cancelled jobs. - ChunkJobCollector.onJobFinished: nullable result, forwarded only when non-null. The sole scheduleTask consumer is collector::onJobFinished, so null results no longer reach the build results list. - awaitCompletion: do not skip stealing cancelled (not started) jobs. - ChunkBuilder.scheduleTask: consumer annotated @nullable. The local Actinium adaptations in RenderSectionManager (cancelled in-flight build handling) and ChunkBuilder are preserved; the ported chunks match upstream f15085d4 verbatim.
1 parent 654486f commit 8707b6b

6 files changed

Lines changed: 53 additions & 42 deletions

File tree

celeritas-common/src/main/java/org/embeddedt/embeddium/impl/render/chunk/RenderSectionManager.java

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,6 @@ private void checkTranslucencyChange() {
269269
private void scheduleTranslucencyUpdates(int camSectionX, int camSectionY, int camSectionZ) {
270270
var renderListManager = this.getCurrentRenderListManager();
271271
var rebuildLists = renderListManager.getRebuildLists().byUpdateType();
272-
var sortRebuildList = rebuildLists.get(ChunkUpdateType.SORT);
273-
var importantSortRebuildList = rebuildLists.get(ChunkUpdateType.IMPORTANT_SORT);
274272
var allowImportant = allowImportantRebuilds();
275273
var translucentPass = this.renderPassConfiguration.defaultTranslucentMaterial().pass;
276274
if (!this.hasTranslucencySortedSections()) {
@@ -317,8 +315,8 @@ private void scheduleTranslucencyUpdates(int camSectionX, int camSectionY, int c
317315

318316
if (cameraChangedSection || section.isAlignedWithSectionOnGrid(camSectionX, camSectionY, camSectionZ)) {
319317
section.setPendingUpdate(update);
320-
// Inject it into the rebuild lists
321-
(update == ChunkUpdateType.IMPORTANT_SORT ? importantSortRebuildList : sortRebuildList).add(section);
318+
// Inject it into the appropriate list
319+
rebuildLists.get(update).add(section);
322320

323321
section.lastCameraX = cameraPosition.x;
324322
section.lastCameraY = cameraPosition.y;
@@ -478,10 +476,25 @@ private boolean rebuildListHasUpdates() {
478476
* Inject sections that requested a rebuild between graph updates into the appropriate rebuild lists.
479477
*/
480478
private void promoteInterimRebuildList() {
479+
if (this.sectionsRequestingUpdate.isEmpty()) {
480+
return;
481+
}
482+
481483
var rebuildLists = this.getCurrentRenderListManager().getRebuildLists().byUpdateType();
484+
boolean graphUpdatePending = this.getCurrentRenderListManager().isNeedsUpdate();
485+
482486
for (var section : this.sectionsRequestingUpdate) {
483-
rebuildLists.get(section.getPendingUpdate()).add(section);
487+
var updateType = section.getPendingUpdate();
488+
if (updateType == null) {
489+
// should never happen, but be defensive
490+
continue;
491+
}
492+
if (!graphUpdatePending || updateType.isImportant()) {
493+
rebuildLists.get(updateType).add(section);
494+
}
484495
}
496+
497+
this.sectionsRequestingUpdate.clear();
485498
}
486499

487500
public void updateChunks(boolean updateImmediately) {
@@ -497,13 +510,7 @@ public void updateChunks(boolean updateImmediately) {
497510
this.builder.tickSchedulingBudget();
498511
}
499512

500-
// Promotion of the interim rebuild list is not required if a graph update is requested, as the graph
501-
// generates a new rebuild list anyway
502-
if (!this.renderListManager.isNeedsUpdate() && !sectionsRequestingUpdate.isEmpty()) {
503-
this.promoteInterimRebuildList();
504-
}
505-
506-
this.sectionsRequestingUpdate.clear();
513+
this.promoteInterimRebuildList();
507514

508515
if (!rebuildListHasUpdates()) {
509516
// Nothing was dispatched, so the workers cannot have been starved for lack of budget.
@@ -873,7 +880,12 @@ protected void scheduleSectionForRebuild(int x, int y, int z, boolean important)
873880
}
874881

875882
if (section.requestUpdate(pendingUpdate) || cancelledInFlightBuild) {
876-
if (!this.getCurrentRenderListManager().isNeedsUpdate() && this.sectionsRequestingUpdate.size() < this.builder.getSchedulingBudget()) {
883+
// Check importance using the section's new update type, as it may not be exactly what we requested
884+
important = section.getPendingUpdate().isImportant();
885+
886+
if (important ||
887+
(!this.getCurrentRenderListManager().isNeedsUpdate() &&
888+
this.sectionsRequestingUpdate.size() < this.builder.getSchedulingBudget())) {
877889
this.sectionsRequestingUpdate.add(section);
878890
} else {
879891
this.markGraphDirty();

celeritas-common/src/main/java/org/embeddedt/embeddium/impl/render/chunk/compile/executor/ChunkBuilder.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import org.apache.logging.log4j.LogManager;
66
import org.apache.logging.log4j.Logger;
77
import org.embeddedt.embeddium.impl.render.chunk.compile.GlobalChunkBuildContext;
8+
import org.jetbrains.annotations.Nullable;
89

910
import java.util.ArrayList;
1011
import java.util.List;
@@ -186,7 +187,7 @@ private void shutdownThreads() {
186187
}
187188

188189
public <TASK extends ChunkBuilderTask<OUTPUT>, OUTPUT> ChunkJobTyped<TASK, OUTPUT> scheduleTask(TASK task, boolean important,
189-
Consumer<ChunkJobResult<OUTPUT>> consumer)
190+
Consumer<@Nullable ChunkJobResult<OUTPUT>> consumer)
190191
{
191192
Objects.requireNonNull(task, "Task must be non-null");
192193

celeritas-common/src/main/java/org/embeddedt/embeddium/impl/render/chunk/compile/executor/ChunkJobCollector.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.embeddedt.embeddium.impl.render.chunk.compile.executor;
22

33
import org.embeddedt.embeddium.impl.render.chunk.compile.ChunkTaskOutput;
4+
import org.jetbrains.annotations.Nullable;
45

56
import java.util.ArrayList;
67
import java.util.List;
@@ -20,9 +21,11 @@ public ChunkJobCollector(int budget, Consumer<ChunkJobResult<? extends ChunkTask
2021
this.collector = collector;
2122
}
2223

23-
public void onJobFinished(ChunkJobResult<? extends ChunkTaskOutput> result) {
24+
public void onJobFinished(@Nullable ChunkJobResult<? extends ChunkTaskOutput> result) {
2425
this.semaphore.release(1);
25-
this.collector.accept(result);
26+
if (result != null) {
27+
this.collector.accept(result);
28+
}
2629
}
2730

2831
public void awaitCompletion(ChunkBuilder builder) {
@@ -31,7 +34,8 @@ public void awaitCompletion(ChunkBuilder builder) {
3134
}
3235

3336
for (var job : this.submitted) {
34-
if (job.isStarted() || job.isCancelled()) {
37+
// Don't try to steal jobs that already started (cancelled jobs are harmless to steal)
38+
if (job.isStarted()) {
3539
continue;
3640
}
3741

celeritas-common/src/main/java/org/embeddedt/embeddium/impl/render/chunk/compile/executor/ChunkJobTyped.java

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@
22

33
import org.embeddedt.embeddium.impl.render.chunk.compile.ChunkBuildContext;
44
import org.embeddedt.embeddium.impl.render.chunk.compile.tasks.ChunkBuilderTask;
5+
import org.jetbrains.annotations.Nullable;
56

67
import java.util.function.Consumer;
78

89
public class ChunkJobTyped<TASK extends ChunkBuilderTask<OUTPUT>, OUTPUT>
910
implements ChunkJob
1011
{
1112
private final TASK task;
12-
private final Consumer<ChunkJobResult<OUTPUT>> consumer;
13+
private final Consumer<@Nullable ChunkJobResult<OUTPUT>> consumer;
1314

1415
private volatile boolean cancelled;
1516
private volatile boolean started;
1617

17-
ChunkJobTyped(TASK task, Consumer<ChunkJobResult<OUTPUT>> consumer) {
18+
ChunkJobTyped(TASK task, Consumer<@Nullable ChunkJobResult<OUTPUT>> consumer) {
1819
this.task = task;
1920
this.consumer = consumer;
2021
}
@@ -31,29 +32,24 @@ public void setCancelled() {
3132

3233
@Override
3334
public void execute(ChunkBuildContext context) {
34-
// Task was cancelled before starting
35-
if (this.cancelled) {
36-
return;
37-
}
38-
39-
this.started = true;
35+
ChunkJobResult<OUTPUT> result = null;
4036

41-
ChunkJobResult<OUTPUT> result;
37+
if (!this.cancelled) {
38+
this.started = true;
4239

43-
long startTime = System.nanoTime();
40+
long startTime = System.nanoTime();
4441

45-
try {
46-
var output = this.task.execute(context, this);
42+
try {
43+
var output = this.task.execute(context, this);
4744

48-
// Task was cancelled while executing
49-
if (output == null) {
50-
return;
45+
// A null output means the task was cancelled while executing
46+
if (output != null) {
47+
result = new ChunkJobResult.Success<>(output, System.nanoTime() - startTime);
48+
}
49+
} catch (Throwable throwable) {
50+
result = new ChunkJobResult.Failure<>(throwable);
51+
ChunkBuilder.LOGGER.error("Chunk build failed", throwable);
5152
}
52-
53-
result = new ChunkJobResult.Success<>(output, System.nanoTime() - startTime);
54-
} catch (Throwable throwable) {
55-
result = new ChunkJobResult.Failure<>(throwable);
56-
ChunkBuilder.LOGGER.error("Chunk build failed", throwable);
5753
}
5854

5955
try {

celeritas-common/src/main/java/org/embeddedt/embeddium/impl/render/chunk/lists/ChunkRebuildLists.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
* @param hasAdditionalUpdates whether there were additional updates not queued for efficiency reasons
1414
*/
1515
public record ChunkRebuildLists(Map<ChunkUpdateType, ArrayDeque<RenderSection>> byUpdateType, boolean hasAdditionalUpdates, Map<ChunkUpdateType, Integer> queueOverflowCounts) {
16-
public static final ChunkRebuildLists EMPTY;
17-
1816
public int getUpdateCount(ChunkUpdateType type) {
1917
return byUpdateType.get(type).size() + queueOverflowCounts.getOrDefault(type, 0);
2018
}
@@ -31,13 +29,13 @@ public boolean isEmpty() {
3129
return true;
3230
}
3331

34-
static {
32+
public static ChunkRebuildLists empty() {
3533
Map<ChunkUpdateType, ArrayDeque<RenderSection>> rebuildLists = new EnumMap<>(ChunkUpdateType.class);
3634

3735
for (var type : ChunkUpdateType.values()) {
3836
rebuildLists.put(type, new ArrayDeque<>());
3937
}
4038

41-
EMPTY = new ChunkRebuildLists(rebuildLists, false, new EnumMap<>(ChunkUpdateType.class));
39+
return new ChunkRebuildLists(rebuildLists, false, new EnumMap<>(ChunkUpdateType.class));
4240
}
4341
}

celeritas-common/src/main/java/org/embeddedt/embeddium/impl/render/chunk/lists/RenderListManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public RenderListManager(SectionGraph graph, boolean shadow, AsyncOcclusionMode
9595
this.async = mode == AsyncOcclusionMode.EVERYTHING || (shadow && mode == AsyncOcclusionMode.ONLY_SHADOW);
9696
this.sectionTicker = sectionTicker;
9797
this.renderLists = SortedRenderLists.empty();
98-
this.rebuildLists = ChunkRebuildLists.EMPTY;
98+
this.rebuildLists = ChunkRebuildLists.empty();
9999
}
100100

101101
/**

0 commit comments

Comments
 (0)