Skip to content

Fix map autosave using server storage - #506

Open
HP-network wants to merge 2 commits into
PaperMC:ver/26.2.xfrom
HP-network:fix/global-map-autosave
Open

HP-network wants to merge 2 commits into
PaperMC:ver/26.2.xfrom
HP-network:fix/global-map-autosave

Conversation

@HP-network

Copy link
Copy Markdown

Description

Map data uses the server-global SavedDataStorage, but Folia autosave was still saving the per-world storage. Move the interval gate and save call to RegionizedServer so the global map data is persisted once per autosave interval.

Testing

  • ./gradlew applyAllPatches --no-daemon --console=plain
  • ./gradlew folia-server:compileJava --no-daemon --console=plain

Both commands pass.

+
+ private void autoSaveMaps(final ServerLevel world) {
+ final int autoSavePeriod = net.minecraft.server.MinecraftServer.getServer().autosavePeriod;
+ private synchronized void autoSaveMaps() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This does not need to be synchronized

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 8e72d12: autoSaveMaps() is now called once from globalTick(long), after all per-world ticks, and the synchronized modifier has been removed.

+ world.moonrise$getChunkTaskScheduler().chunkHolderManager.processTicketUpdates(); // required to eventually process ticket updates
+
+ this.autoSaveMaps(world);
+ this.autoSaveMaps();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is being called inside the tick for worlds. This means that this gets called 1 time for every world in the server. This call should be moved OUTSIDE the method for ticking specific worlds global tick

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 8e72d12: autoSaveMaps() is now called once from globalTick(long), after all per-world ticks, and the synchronized modifier has been removed.

@HP-network

Copy link
Copy Markdown
Author

Updated in 8e72d12.

  • Moved map autosave out of the per-world tick and into the global tick after all worlds have been processed.
  • Removed the unnecessary synchronized modifier from autoSaveMaps().

applyAllPatches and folia-server:compileJava both pass.

@HP-network

Copy link
Copy Markdown
Author

@Dueris I moved autoSaveMaps() out of the per-world tick and into globalTick() after all worlds have been processed, and removed the synchronized modifier in 8e72d12. Both applyAllPatches and folia-server:compileJava pass on the updated head. Could you take another look and resolve the two inline threads if this addresses your concerns? Thanks.

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