Skip to content

memory-fix - #793

Merged
louis-e merged 4 commits into
louis-e:mainfrom
pingpongmury:memfix
Mar 26, 2026
Merged

louis-e merged 4 commits into
louis-e:mainfrom
pingpongmury:memfix

Conversation

@pingpongmury

Copy link
Copy Markdown
Contributor

save chunks as they're completed rather than keeping them in memory and saving them at the end of ground generation. Results in far lower memory footprint and enables generating much larger areas.

save chunks as they're completed rather than keeping them in memory and
saving them at the end of ground generation. Results in far lower memory
footprint and enables generating much larger areas.
@pingpongmury

Copy link
Copy Markdown
Contributor Author

retrigger-benchmark

@github-actions

Copy link
Copy Markdown

⏱️ Benchmark run finished in 0m 28s
🧠 Peak memory usage: 1115 MB

📈 Compared against baseline: 30s
🧮 Delta: -2s
🔢 Commit: d605b7f

🟢 Generation time is unchanged.

📅 Last benchmark: 2026-03-16 23:25:58 UTC

You can retrigger the benchmark by commenting retrigger-benchmark.

@louis-e

louis-e commented Mar 26, 2026

Copy link
Copy Markdown
Owner

Hey there, thanks a lot for the contribution! Looks good, I just need to fix a few things. Will merge soon

louis-e added 3 commits March 26, 2026 18:47
Updated save_java function to ensure metadata is saved even if all regions were flushed during ground generation.
Added error handling for saving regions with user messages.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces peak memory usage during Java world generation by flushing completed regions to disk incrementally during the ground-generation pass, instead of retaining all modified regions in memory until the final save step.

Changes:

  • Add WorldEditor::flush_region() to serialize a single Java Anvil region and drop it from self.world.regions.
  • Update Java saving to still write metadata even when all regions have already been flushed, and to skip region-writing when nothing remains in memory.
  • Call flush_region() from the ground-generation loop once a full region column (by region_x) has been completed.

Reviewed changes

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

File Description
src/world_editor/mod.rs Introduces flush_region() to incrementally serialize and evict Java regions from memory.
src/world_editor/java.rs Ensures metadata is saved even if all regions were flushed; exposes save_single_region to the parent module; short-circuits when no regions remain.
src/data_processing.rs Flushes completed region columns during ground generation based on chunk iteration progress.

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

Comment thread src/world_editor/mod.rs
Comment thread src/data_processing.rs
@louis-e
louis-e merged commit a53ba4a into louis-e:main Mar 26, 2026
5 of 6 checks passed
louis-e added a commit that referenced this pull request Mar 26, 2026
…data

When flush_region() saves a region to disk and removes it from memory,
subsequent block writes (e.g. tree canopy leaves crossing a region
boundary) would silently re-create the region via entry().or_default().
The re-created region contained only the stray blocks, and save() would
then overwrite the correct .mca file with this near-empty one — causing
entire regions to appear missing in the generated world.

Fix: track flushed region coordinates in a HashSet and skip all write
operations (set_block, set_block_with_properties, set_block_if_absent,
fill_column) targeting already-flushed regions. The skipped writes are
cosmetic (tree leaves at region edges) so there is no visual impact.

Closes the "partially empty world" bug introduced by the incremental
region flushing in PR #793.
louis-e added a commit that referenced this pull request Mar 26, 2026
…rites

Two fixes for the incremental region flushing introduced in PR #793:

1. Delay flush by one chunk past region boundaries so tree canopies
   (up to 3 blocks spill radius) are fully written before serialization.

2. Track flushed regions and purge any stale re-created entries at save
   time, preventing near-empty regions from overwriting correct .mca files.

No hot-path changes — the tracking is a single HashSet insert per flush,
and purge runs once at save time.
louis-e added a commit that referenced this pull request Mar 26, 2026
The incremental flush_region approach serialized regions sequentially
during ground generation instead of in parallel at save time. Benchmarks
show this causes a consistent ~47% time regression (30s → 44s) while
actually INCREASING peak memory (~935 MB → ~1060 MB) due to
serialization buffer overhead.

The memory reduction only helps for extremely large areas where in-memory
region data alone exceeds available RAM, but for those areas the time
penalty is proportionally worse (linear scaling).

This reverts the flush_region calls and all associated bookkeeping
(flushed_regions tracking, purge_stale_regions, delayed flush). The
flush_region method itself is removed as dead code. Regions are kept
in memory during generation and saved in parallel via rayon in
save_java(), restoring the original performance characteristics.
@pingpongmury

Copy link
Copy Markdown
Contributor Author

Looks like this turned into a bit of a mess - my apologies. I actually thought I was making the PR to the main branch of my fork so I could make use of your benchmarking action.

During my testing, my RAM usage dropped from more than the 32GB in my system (causing a hang and crash) to about 8-9GB during ground generation. I still saw peaks of 24+GB while processing terrain, though. I was able to generate a roughly 350sq. mile area without issue (at least that I noticed in-game).

Regardless, I appreciate you taking a look! We just got my dad into Minecraft and I used Arnis to generate his entire home-county. I showed him mountains in Colorado, and let him explore NYC. It has been blowing his mind, ha. You guys have built a supremely awesome tool and I sincerely appreciate your work.

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.

3 participants