Write configs asynchronously, first part of async API rewrite #1510
Conversation
|
To prevent double writes, any queued io operations should have a cancellation token before committing and care must be taken before creating a new write task to make sure that only one queued write exists at the same time. An example of that would be an older write operation overwriting a newer one if its thread had to be stalled for any reason |
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| await this.configs.SaveAsync(currentConfig, this.plugin.InternalName, this.plugin.Manifest.WorkingPluginId); | ||
| #pragma warning restore CS0618 // Type or member is obsolete |
There was a problem hiding this comment.
These pragmas are unnecessary, SaveAsync is not obsolete.
| @@ -508,7 +514,8 @@ private void Save() | |||
| { | |||
| ThreadSafety.AssertMainThread(); | |||
|
|
|||
| Service<ReliableFileStorage>.Get().WriteAllText( | |||
| this.writeTask?.Wait(); | |||
| this.writeTask = Service<ReliableFileStorage>.Get().WriteAllTextAsync( | |||
| this.configPath, JsonConvert.SerializeObject(this, SerializerSettings)); | |||
| this.DalamudConfigurationSaved?.Invoke(this); | |||
There was a problem hiding this comment.
We'll be invoking DalamudConfigurationSaved before the save actually completes. This seems fine, given that the only use I can see is updating DalamudPluginInterface#GeneralChatType, but it does feel like a behavioral change that should at least be noted.
Are you referring to something specific, or are you saying that the system should handle this? I'm waiting for these tasks in the two spots I'm using this now before kicking off another, so this shouldn't be an issue, error handling isn't great but I wanted to get this in for the patch. I'll go over it again when this is going to be a public API. |
Should fix some microstuttering complaints we have been getting. Reads aren't async yet, they definitely should be, but they should take the same code path as before the VFS in 99.9% of cases so it doesn't matter at the moment.