Prime Terra cache before tpll teleports by robinferch · Pull Request #79 · BTE-Germany/TerraPlusMinus

robinferch · 2026-06-02T19:31:32Z

Summary

add a public Terraplusminus API for checking and priming /tpll target regions
wait for a 16-chunk-radius Terra-- cache prewarm before standalone /tpll teleports
add retry/invalidation around failed Terra-- chunk data loads
bound prewarm concurrency to avoid unbounded Terra-- loader fanout

Verification

mvn -q -DskipTests package

Copilot

Pull request overview

This PR adds a public API to check/prime Terra-- chunk-data cache for a target teleport region, integrates a 16-chunk-radius prewarm into /tpll teleports, and adds retry/invalidation plus bounded concurrency when loading Terra-- chunk data.

Changes:

Expose Terraplusminus APIs to check whether a teleport region is generated and to prime Terra-- cache for that region.
Add cache prewarm + user messaging before executing /tpll teleports (including linked-world teleports).
Add retry + cache invalidation for failed Terra-- chunk-data loads, and batch prewarming to bound concurrent loads.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`src/main/java/de/btegermany/terraplusminus/Terraplusminus.java`	Adds public region-generation check and cache-priming/prepare APIs for optional integrations.
`src/main/java/de/btegermany/terraplusminus/commands/TpllCommand.java`	Wraps teleport execution with region prewarm logic (16-chunk radius) and improved main-thread scheduling.
`src/main/java/de/btegermany/terraplusminus/gen/RealWorldGenerator.java`	Introduces chunk-data retry/invalidation and a bounded-concurrency cache prewarm API; adds `isParallelCapable()`.
`src/main/java/de/btegermany/terraplusminus/gen/tree/TreePopulator.java`	Refactors chunk-data loading to an injected provider and updates biome-based tree selection + RNG handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Zoriot · 2026-06-05T23:41:46Z

    // <editor-fold desc="Constants and Fields">
    public static final String LAT_LON_HEIGHT = "latLonHeight";
    public static final String TPLL_OTHERS_PERMISSION = "t+-.forcetpll";
+    private static final int TELEPORT_PREP_RADIUS_CHUNKS = 16;


Please make that configurable

This default value also seems quite high. The default render distance spherical radius is 10, and with a square radius of 16 this generates approximately (32²)/(pi*10²) = 3.26 times the surface it will actually need to load after the teleportation.

An alternative from having it configurable would be to have it match the render distance radius.

Zoriot · 2026-06-06T21:24:48Z

+     * This method is intentionally public so optional teleportation plugins can call it without
+     * Terraplusminus depending on them.
+     */
+    public boolean isTeleportRegionGenerated(@NotNull World world, double x, double z, int radiusChunks) {


As i have suspected this call is relative resource intense
https://mclo.gs/s7oXM5W
I would prefer to only check the chunk where you are teleporting to or make it opt in/out & configurable

Generally performance seems not to be that much better, not not done extensive testing trough.

Zoriot · 2026-06-06T21:27:48Z

+     * Primes Terra-- chunk data for the requested teleport region when the world uses Terraplusminus.
+     * Non-Terraplusminus worlds complete immediately so this API is safe for optional integrations.
+     */
+    public @NotNull CompletableFuture<Void> primeTeleportRegionCache(@NotNull World world, double x, double z, int radiusChunks) {


Both of these method shouldn't be in that class. Imo it should be also abstracted as a real api

SmylerMC

What exactly is this PR trying to achieve? Its description explains what it changes but not why, and I wonder how much of it actually improves performance and how much of it is premature optimization. World generation within the plugin goes through multiple abstraction layers, multiple of which have their own concurrency or caching systems. This makes things quite hard to grasp, and mandating rigorous goals and benchmarking when introducing new optimization mechanisms. Otherwise, we risk adding complexity for no actual benefit. The fact this PR also includes changes that not directly related to its main point (warming the cache) also makes it harder to review.

SmylerMC · 2026-06-03T07:19:41Z

                BlockData surfaceBlock = this.blockMapper.map(terraData.surfaceBlock(x, z));
                if (surfaceBlock == null) {
+                    // We do that for each column, so it does not depend on the configuration but only on the seed
+                    int startMountainHeight = random.nextInt(7500, 7520);


As hinted by the comment, moving this random computation after the vertical bounds early return changes the generation behavior as the random value picked for startMountainHeight now depends on both the world seed and which columns were skipped. Ideally, randomization should only depend on the seed. For example, worlds with different Y offsets but otherwise identical configurations will have different mountain patterns.

SmylerMC · 2026-06-21T07:57:44Z

    // <editor-fold desc="Constants and Fields">
    public static final String LAT_LON_HEIGHT = "latLonHeight";
    public static final String TPLL_OTHERS_PERMISSION = "t+-.forcetpll";
+    private static final int TELEPORT_PREP_RADIUS_CHUNKS = 16;


This default value also seems quite high. The default render distance spherical radius is 10, and with a square radius of 16 this generates approximately (32²)/(pi*10²) = 3.26 times the surface it will actually need to load after the teleportation.

An alternative from having it configurable would be to have it match the render distance radius.

SmylerMC · 2026-06-21T08:23:57Z

+        int radius = Math.max(0, radiusChunks);
+        CompletableFuture<Void> result = CompletableFuture.completedFuture(null);
+        List<ChunkPos> batch = new ArrayList<>(PRIME_CACHE_CONCURRENCY);
+        for (int chunkX = centerChunkX - radius; chunkX <= centerChunkX + radius; chunkX++) {


Optimization suggestion: make this a disk instead of a square, to reduce the area by a factor of (2r)² / (pi*r)² = 1.3.

SmylerMC · 2026-06-21T08:38:23Z

@@ -204,7 +215,92 @@ private CachedChunkData getTerraChunkData(int chunkX, int chunkZ) {
     * @return A CompletableFuture containing the CachedChunkData
     */
    public CompletableFuture<CachedChunkData> getBaseHeightAsync(int chunkX, int chunkZ) {


Out of scope for this PR, but this method is badly named.

SmylerMC · 2026-06-21T08:58:18Z

+        }).thenCompose(result -> result);
+    }
+
+    private CompletableFuture<CachedChunkData> retryChunkData(ChunkPos chunkPos, int attempt, Throwable throwable) {


Issue: a retry mechanism seems out of scope for this PR, and this specific implementations is likely to cause retry-storms.

SmylerMC · 2026-06-21T09:21:15Z

+        for (int chunkX = centerChunkX - radius; chunkX <= centerChunkX + radius; chunkX++) {
+            for (int chunkZ = centerChunkZ - radius; chunkZ <= centerChunkZ + radius; chunkZ++) {
+                batch.add(new ChunkPos(chunkX, chunkZ));
+                if (batch.size() >= PRIME_CACHE_CONCURRENCY) {


Question: is the addition of this batching mechanism the result of some benchmarking? If so, explaining why it is necessary in a comment would be nice. If not, it looks like it could be premature optimization.

SmylerMC · 2026-06-21T09:27:10Z

+     */
+    public CompletableFuture<Void> primeCache(int centerChunkX, int centerChunkZ, int radiusChunks) {
+        int radius = Math.max(0, radiusChunks);
+        CompletableFuture<Void> result = CompletableFuture.completedFuture(null);


Suggestion: using a semaphore here instead of this CompletableFuture chaining would likely improve readability.

SmylerMC · 2026-06-21T09:28:15Z

+        List<ChunkPos> chunkBatch = List.copyOf(batch);
+        return previous.thenCompose(unused -> CompletableFuture.allOf(
+                chunkBatch.stream()
+                        .map(chunkPos -> this.getChunkDataAsync(chunkPos).thenApply(data -> null))


.thenApply(data -> null) is unnecessary

SmylerMC · 2026-06-21T09:36:12Z



-            } catch (InterruptedException | ExecutionException e) {
+            } catch (InterruptedException e) {


Question: why handle InterruptedException separately?

SmylerMC · 2026-06-21T09:37:54Z

+import java.util.function.Function;


 public class TreePopulator extends BlockPopulator {


Sharing the chunk data provider with the world generator is a great change for this PR, but there is also lots of refactoring in this class that seems out of scope.

robinferch added 2 commits June 2, 2026 21:30

Prime Terra cache before tpll teleports

9213f59

Fix tree biome lookup for CI build

855f0c3

robinferch requested review from Zoriot and Copilot June 2, 2026 19:58

Copilot started reviewing on behalf of robinferch June 2, 2026 19:58 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

Comment thread src/main/java/de/btegermany/terraplusminus/gen/tree/TreePopulator.java

Comment thread src/main/java/de/btegermany/terraplusminus/gen/RealWorldGenerator.java

Zoriot requested changes Jun 6, 2026

View reviewed changes

SmylerMC reviewed Jun 21, 2026

View reviewed changes



		} catch (InterruptedException \| ExecutionException e) {
		} catch (InterruptedException e) {

		import java.util.function.Function;


		public class TreePopulator extends BlockPopulator {

Uh oh!

Conversation

robinferch commented Jun 2, 2026

Summary

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zoriot Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SmylerMC left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Zoriot Jun 6, 2026 •

edited

Loading