move SavedData.save to IO worker to avoid synchronous IO on main thread

[thiakil]

The new Atomic Move changes to SavedData are causing synchronous IO on the main/server thread, even when saving should be asynchronous.
This is adds up in large modpacks

Ref 1 2

This PR moves that to an IO worker and adds waiting in the appropriate places.

[neoforged-pr-publishing]

• Publish PR to GitHub Packages

[Technici4n]

Does vanilla not perform this IO on the main thread already?

[Technici4n]

We need to be a bit careful I think:

• Moving IO to a separate thread just to block until completion will not improve global performance unless IO is performed in parallel.
• It looks like the bottleneck is the actual disk write. Doing that in parallel or on a different thread shouldn't bring improvements?

[thiakil]

Does vanilla not perform this IO on the main thread already?

Yes, but it does not force a flush to disk and an atomic move. Plus scaling.

to block until completion

Not quite what's happening. That's only when /save-all flush is used. Normally the io will complete in the background, which is why only one thread

[marchermans]

I agree with @Technici4n That we need to be majorly carefull here.

We introduced the default always flush/atomic-move behaviour to make sure that capabilities and modded savedata are always written.
This change in and of itself seems to be fine, but future changes might re-open that can of worms.

[Technici4n]

Thanks for the explanations. Moving IO off-thread is quite nice since we are already doing atomic moves that should be safe to perform concurrently.

[XFactHD]

Finally managed to take a closer look at this. The proposed solution is fine and my initial worries turned out to be completely unjustified. However, I would like to simplify the implementation by making it closer to what Mojang did in 1.21.4: Instead of spinning up a custom executor, manually adding the tasks and then shutting it down to wait on the remaining tasks in case of a flush, the individual IO tasks can be chained with CompletableFuture#thenRunAsync() using the Util.ioPool() executor (non-daemon, so it's safe to use for critical stuff like this and it's what vanilla uses in 1.21.4), storing the latest future in a field and then joining on it when necessary. If desired, the last future can also be cleared after then join by setting the field to CompletableFuture.completedFuture(null).

[Technici4n]

LGTM

[neoforged-releases]

🚀 This PR has been released as NeoForge version 21.1.118.