search

openmod / openmod

OpenMod .NET Plugin Framework
https://openmod.github.io/openmod-docs/
Other
269 stars 43 forks source link

Player death causes large ping spikes when having a large openmod.users.yaml #537

Closed DronXa closed 2 years ago

DronXa commented 3 years ago

Player death causes large ping spikes when having a large openmod.users file. This has been tested with a 650KB user file. After deleting the file the lag spikes are resolved. The only openmod plugins that are used is Newessentials, OpenDeathMessages and ChatAnnoyer.

Any type of player death causes the ping spikes, even suicides.

DronXa commented 3 years ago

Newessentials saves playerdeath in openmod.users file, this issue might be related to player data storage in openmod.users.

Trojaner commented 3 years ago

Yeah, we should fix this at the root by figuring out why deserialization is so slow. Its prob. because we chose yaml over json

rube200 commented 3 years ago

O^2 loop https://github.com/openmod/openmod/blob/1f48d4ebc560529b53586f072649cf494c565373/framework/OpenMod.Core/Users/UserDataStore.cs#L209-L215

Another O loop https://github.com/openmod/openmod/blob/53c2ef9c8cb27a764f177a9700e8521089cc0dd1/framework/OpenMod.Core/Persistence/YamlDataStore.cs#L163

ComputeHash https://github.com/openmod/openmod/blob/53c2ef9c8cb27a764f177a9700e8521089cc0dd1/framework/OpenMod.Core/Persistence/YamlDataStore.cs#L172

Another ComputeHash https://github.com/openmod/openmod/blob/53c2ef9c8cb27a764f177a9700e8521089cc0dd1/framework/OpenMod.Core/Persistence/YamlDataStore.cs#L182

FileWrite that can be slow in some disks https://github.com/openmod/openmod/blob/53c2ef9c8cb27a764f177a9700e8521089cc0dd1/framework/OpenMod.Core/Persistence/YamlDataStore.cs#L203

Conclusion: O^3 loops 2 Hashes 1 Write

With alot of data it is expected a decrease of performanced

rube200 commented 3 years ago

Sugestion: make YamlDataStore.SaveAsync run in another thread since it already uses locks

iamsilk commented 3 years ago

Thread-locking locks should not be used. Should change to using Nito.AsyncEx's AsyncLocks.

File.WriteAllBytes will lock the thread as well, so this should transfer to another thread as well.

Trojaner commented 3 years ago

File.WriteAllBytes is acceptable in this case, it shouldn't block for too long in the majority of cases. Unfortunately the built-in async version of it was not available last time I checked, but we could write our own async version with TaskCompletionSource and QueueUserWorkItem

ComputeHash isnt causing it, this issue existed long before it was added

rube200 commented 2 years ago

No updates on this, and already have a pr associated