Skip to content

Snapshots for GenTerraPostProcess - #289

Merged
trevorftp merged 2 commits into
StratumServer:indevfrom
tehtelev:gen-post
Sep 5, 2026
Merged

Snapshots for GenTerraPostProcess#289
trevorftp merged 2 commits into
StratumServer:indevfrom
tehtelev:gen-post

Conversation

@tehtelev

@tehtelev tehtelev commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

We've added a thread-safe snapshot system to GenTerraPostProcess, which caches subchunk data in local arrays once before scanning. This dramatically speeds up world generation: millions of slow GetBlockIdUnsafe calls with palette unpacking are replaced with instant index reads. We've also implemented strict array bounds checking and NullReferenceException protection, completely eliminating rare but critical generator crashes caused by non-standard column heights or missing data in chunks.

Type

  • Bug fix
  • Performance
  • New feature
  • Refactor or cleanup
  • Docs or build

Checklist

  • .\scripts\extract-patches.ps1 ran clean.
  • dotnet build VintageStory.slnx -c Release is green.
  • Every vanilla edit has a // Stratum marker.
  • No vanilla source committed.
  • Tested on a real server start, not just compilation.

Performance numbers

Tested on the generation of 31417 chunk columns (6 threads) by /stratum pregen start radius 100 16000 16000
Seed - seed 1027995113.
World setting - default.
AVX instructions: enabled

Three runs were performed before and after the changes using the Jetbrains dotTrace program.

Before

  • OnChunkColumnGen method execution time: 5387 ms; 5529 ms; 5561 ms.
  • Average: 5492.3 ms
  • Standard deviation: 92.6 ms

After

  • OnChunkColumnGen method execution time: 3402 ms; 3271 ms; 3681 ms
  • Average: 3451.3 ms
  • Standard deviation: 209.4 ms

Summary: The snapshot optimization resulted in an average speedup of ~37.2% (dropping execution time from ~5.5 seconds down to ~3.45 seconds). However, the standard deviation increased from 92.6 ms to 209.4 ms, indicating that the new version shows less stable chunk generation performance compared to the baseline.

@tehtelev
tehtelev marked this pull request as ready for review August 30, 2026 16:38
+ }
+ else
+ {
+ Array.Clear(stratumChunkData[cy], 0, stratumChunkData[cy].Length);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think missing chunk data should become air here. That changes the worldgen result and can feed fake air into the floating-block cleanup. Better to skip/fail this column than guess.

{
int x = baseindex3d % chunksize;
int z = baseindex3d / chunksize;
if (!chunkVisitedNodes.Contains(index3d)) deletePotentialFloatingBlocks(chunkX * chunksize + x, baseY + y, chunkZ * chunksize + z);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we recheck the seed live before calling this? As far as I can tell the snapshot can be stale after earlier cleanup and deletePotentialFloatingBlocks assumes the starting block is still solid

+ // Stratum: protect against NRE if chunks[cy] or its Data is null
+ if (chunks[cy]?.Data != null)
+ {
+ chunks[cy].Data.CopyBlocksToUnsafe(stratumChunkData[cy]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CopyBlocksToUnsafe is explicitly the no-lock path we need a synchronized bulk-copy path here

@tehtelev

tehtelev commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, these are all good catches. Addressed all three points below. Summary of the key trade-off: the hot flood-fill loop works entirely over int[] snapshots and performs zero per-cell accessor lookups, so adding a single live re-check before cleanup costs nothing performance-wise.

1 - Missing chunk data becomes air
Agreed this shouldn't alter generation results. I kept the Array.Clear (fill with air = 0) strictly as a crash-protection fallback and documented it as such: at this late stage of column generation a null subchunk should not occur in practice, so we do not try to fabricate terrain — we just avoid an NRE on a missing provider. See BuildColumnSnapshot.

2 - Recheck the seed live before cleanup
Done with a single GetBlockId call at the call site (IsSeedBlockStillSolid) right before entering BFS. This is one accessor lookup, not a per-block loop, so it does not reintroduce the O(volume) accessor cost and preserves the speedup. I chose to keep the guard at the call site rather than moving it inside deletePotentialFloatingBlocks, since the latter would pay that check on every invocation. If you'd prefer the same protection inside the method, I can move it — happy to do whichever reads better for the codebase.

3 - CopyBlocksToUnsafe has no lock
Wrapped the CopyBlocksToUnsafe fast path in TakeBulkReadLock() / ReleaseBulkReadLock(), released in a finally. Reads are guarded against concurrent chunk-data reloads; the read lock is uncontended per column. See BuildColumnSnapshot.

@tehtelev
tehtelev requested a review from trevorftp September 4, 2026 17:33
@trevorftp
trevorftp merged commit 6650753 into StratumServer:indev Sep 5, 2026
trevorftp added a commit that referenced this pull request Sep 6, 2026
* Fixing allocations inside ScheduleReadyTasks

* Corrections for #273

* Improvements to AVX instructions in ChunkDataLayer

* GenTerra.stratumGenerate optimizations

* Fix 279

* Return to Floor rounding for identity

* Closing issues with parity testing and preventing concurrent execution of the vanilla path

* Fixes 1 and 2 for 279 v2

* Caves first edit

* Caves delayed writes

* Fix 279 v3

* Fix 285

* Fix 285 v2

* Fix 285 v3

* Fixed: Boats and rafts not responding to controls on servers

* Fix 285 v4

* Fix 285 v5

* Fix 286

* Fix 285 v6

* Fix 288

* Fix 285 v7

* Skip server-side pose matrices until something reads them (#291)

* Skip server-side pose matrices until something reads them

Every player has requirePosesOnServer set, so ServerAnimator recomputed
the whole seraph skeleton every tick for every player, and for every
dead entity, while the server only reads poses through
GetAttachmentPointPose and GetPosebyName in a handful of places.
Mark the poses stale in calculateMatrices and recompute on the first
read. Config: Performance.EntityTicking.LazyServerPoses (default true).

Also replace the LINQ Any in AnimationManager.OnServerTick with a loop;
it allocated an enumerator and a closure per entity per tick.

Micro-benchmark on seraph.json, VintagestoryAPI 1.22.7: 5.5 to 7.4 us
per player per tick with matrices, 0.01 to 0.14 us without. At 650
players that is 3.6 to 4.8 ms per tick, 30 to 40 percent of the
entity.tick.players time in the 650-bot timings report. Lazy and eager
poses compared over 6000 frames with random animation changes: 1842
attachment point reads, zero difference.

* Lock the frame update with the lazy recompute, clear stale on eager path

Review fixes for #291. OnFrame now holds the same lock as the lazy
recompute so a physics-thread reader cannot rebuild poses from animation
state the main thread is advancing. The eager path clears the stale flag
so switching lazy poses off at runtime does not trigger one extra
recompute per entity. The AnimationManager comment now says what the
LINQ call actually cost: a boxed enumerator, not a closure.

* Make the stale pose flag volatile

The first check in StratumEnsurePosesFresh runs outside the lock as a
fast path. Without volatile a physics-thread reader could miss the main
thread setting the flag and hand out stale poses. Review fix for #291.

* Fix /kitedit registration error, add snapshot scope, and document kit commands (#294)

Fix /kitedit registration error, add snapshot scope, and document kit commands (#293, #295)

* Snapshots for GenTerraPostProcess (#289)

* Snapshots for GenTerraPostProcess

* Fix 289

* Fixed: Prevent view distance griefing and invalid block break modes

* Fixed: Close block interaction and mount movement exploits

* Vectorizing SetLightBulkUnsafe (#296)

* Vectorizing SetLightBulkUnsafe

* Some improvements for 296

---------

Co-authored-by: tehtelev <tehtelev@gmail.com>
Co-authored-by: tehtelev <50070668+tehtelev@users.noreply.github.com>
Co-authored-by: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com>
Co-authored-by: Michael Andrzejewski <55041358+Michael-Andrzejewski@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants