Paper: Main thread playerdata saving on slower drives

[ghost]

Timings or Profile Screenshot:

___We ask that all timings/profiles are a screenshot, not a link. Links inhibit our ability to figure out the real cause of the issue.___
https://imgur.com/a/Dgg2bTB

Description of issue:

___If applicable___
Paper really shouldn't be doing IO on the main thread. Slower drives generate significant lag spikes every time players are saved. Paper should have an IO thread and queue when it comes to saving player data. If it is absolutely necessary for some plugins to save player data urgently then methods such as player.savedata() should be an exception to this.

Paper version:

git paper 133 (1.16.1)

[aikar]

This is unlikely to ever be solved. This is a super complicated problem that is not as simple as "just do it async!" it requires careful considerations for pending saves and load order, to ensure that stale data is not loaded.

Right now a good bit of this all depends on main thread.

Yes it would be ideal, but it's not going to be worked on anytime soon.

[MrIvanPlays]

at least its not like when we didn't have incremental player saving (100ms of the tick to save 40 players :eyes:)

[ghost]

https://github.com/YatopiaMC/Yatopia/blob/ver/1.16.3/patches/server/0010-Player-saving-async-FileIO.patch this problem has been fixed by this YatopiaMC patch. Ideally this should be ported to paper

[electronicboy]

That patch is inherently dangerous

On 24 Sep 2020, at 20:34, dudeman123 [e-mail] wrote:

https://github.com/YatopiaMC/Yatopia/blob/ver/1.16.3/patches/server/0010-Player-saving-async-FileIO.patch https://github.com/YatopiaMC/Yatopia/blob/ver/1.16.3/patches/server/0010-Player-saving-async-FileIO.patch this problem has been fixed by this YatopiaMC patch. Ideally this should be ported to paper

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub https://github.com/PaperMC/Paper/issues/4209#issuecomment-698545339, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAZDQDJVIWADZMS4GI7DSHONLHANCNFSM4QLIRTFQ.

[MrIvanPlays]

@electronicboy we have certain measures in place so data loss and desync doesn’t happen. If you think these smol changes are unsafe then you should rethink if ur async chunk io is safe.

[aikar]

I'm about ready to ban you for that comment.

Our changes actually properly implement the concept of async saving. That patch does not.
It's way more complicated than this patch linked does.

That patch is not safe.

[aikar]

Mojang couldn't get this code right for like 8 years for chunks, to the point they moved chunk saving to do IO fully synchronous in early 1.14 development... There are serious race conditions to consider here.

[MrIvanPlays]

u shouldnt compare mojang to u. is all bad engineers. if they had good ones it would already be done and we wont have such issues.