Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write configs asynchronously, first part of async API rewrite #1510

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion Dalamud/Configuration/Internal/DalamudConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Globalization;
using System.IO;
using System.Linq;
using System.Threading.Tasks;

using Dalamud.Game.Text;
using Dalamud.Interface.Internal.Windows.PluginInstaller;
Expand Down Expand Up @@ -39,6 +40,8 @@ internal sealed class DalamudConfiguration : IServiceType, IDisposable
[JsonIgnore]
private bool isSaveQueued;

private Task? writeTask;

/// <summary>
/// Delegate for the <see cref="DalamudConfiguration.DalamudConfigurationSaved"/> event that occurs when the dalamud configuration is saved.
/// </summary>
Expand Down Expand Up @@ -488,6 +491,9 @@ public void Dispose()
{
// Make sure that we save, if a save is queued while we are shutting down
this.Update();

// Wait for the write to finish
this.writeTask?.Wait();
}

/// <summary>
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

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.

}
Expand Down
28 changes: 26 additions & 2 deletions Dalamud/Configuration/PluginConfigurations.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO;
using System.Threading.Tasks;

using Dalamud.Storage;
using Newtonsoft.Json;
Expand All @@ -11,6 +12,7 @@ namespace Dalamud.Configuration;
public sealed class PluginConfigurations
{
private readonly DirectoryInfo configDirectory;
private Task? currentWriteTask;

/// <summary>
/// Initializes a new instance of the <see cref="PluginConfigurations"/> class.
Expand All @@ -32,10 +34,32 @@ public PluginConfigurations(string storageFolder)
/// <param name="config">Plugin configuration.</param>
/// <param name="pluginName">Plugin name.</param>
/// <param name="workingPluginId">WorkingPluginId of the plugin.</param>
[Obsolete("Use SaveAsync instead.")]
public void Save(IPluginConfiguration config, string pluginName, Guid workingPluginId)
{
Service<ReliableFileStorage>.Get()
.WriteAllText(this.GetConfigFile(pluginName).FullName, SerializeConfig(config), workingPluginId);
// TODO(api10): This API should be async-only. Remove the sync version!
// Yes, this means that all plugins will be blocking each other when writing configs for now, but that's fine until
// we can make this API async
this.currentWriteTask?.Wait();
this.currentWriteTask = Service<ReliableFileStorage>.Get()
.WriteAllTextAsync(this.GetConfigFile(pluginName).FullName, SerializeConfig(config), workingPluginId);
}

/// <summary>
/// Save/Load plugin configuration.
/// NOTE: Save/Load are still using Type information for now,
/// despite LoadForType superseding Load and not requiring or using it.
/// It might be worth removing the Type info from Save, to strip it from all future saved configs,
/// and then Load() can probably be removed entirely.
/// </summary>
/// <param name="config">Plugin configuration.</param>
/// <param name="pluginName">Plugin name.</param>
/// <param name="workingPluginId">WorkingPluginId of the plugin.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task SaveAsync(IPluginConfiguration config, string pluginName, Guid workingPluginId)
{
await Service<ReliableFileStorage>.Get()
.WriteAllTextAsync(this.GetConfigFile(pluginName).FullName, SerializeConfig(config), workingPluginId);
}

/// <summary>
Expand Down
22 changes: 11 additions & 11 deletions Dalamud/Game/Text/SeStringHandling/SeString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -368,17 +368,6 @@ public static SeString CreateMapLinkWithInstance(uint territoryId, uint mapId, i
return null;
}

private static string GetMapLinkNameString(string placeName, int? instance, string coordinateString)
{
var instanceString = string.Empty;
if (instance is > 0 and < 10)
{
instanceString = (SeIconChar.Instance1 + instance.Value - 1).ToIconString();
}

return $"{placeName}{instanceString} {coordinateString}";
}

/// <summary>
/// Creates an SeString representing an entire payload chain that can be used to link party finder listings in the chat log.
/// </summary>
Expand Down Expand Up @@ -512,4 +501,15 @@ public override string ToString()
{
return this.TextValue;
}

private static string GetMapLinkNameString(string placeName, int? instance, string coordinateString)
{
var instanceString = string.Empty;
if (instance is > 0 and < 10)
{
instanceString = (SeIconChar.Instance1 + instance.Value - 1).ToIconString();
}

return $"{placeName}{instanceString} {coordinateString}";
}
}
2 changes: 1 addition & 1 deletion Dalamud/Interface/Internal/DalamudInterface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ public void OpenPluginStats()
/// <summary>
/// Opens the <see cref="PluginInstallerWindow"/> on the plugin installed.
/// </summary>
/// <param name="kind">The page of the installer to open.</param>
public void OpenPluginInstaller()
{
this.pluginWindow.OpenTo(this.configuration.PluginInstallerOpen);
Expand Down Expand Up @@ -394,6 +393,7 @@ public void ToggleDataWindow(string dataKind = null)
/// <summary>
/// Toggles the <see cref="PluginInstallerWindow"/>.
/// </summary>
/// <param name="kind">The page of the installer to open.</param>
public void TogglePluginInstallerWindowTo(PluginInstallerWindow.PluginInstallerOpenKind kind) => this.pluginWindow.ToggleTo(kind);

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions Dalamud/Interface/Internal/Windows/Data/DataWindow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ internal class DataWindow : Window
new DataShareWidget(),
new NetworkMonitorWidget(),
new IconBrowserWidget(),
new VfsWidget(),
};

private readonly IOrderedEnumerable<IDataWindowWidget> orderedModules;
Expand Down
102 changes: 102 additions & 0 deletions Dalamud/Interface/Internal/Windows/Data/Widgets/VfsWidget.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
using System.Diagnostics;
using System.IO;

using Dalamud.Configuration.Internal;
using Dalamud.Storage;
using ImGuiNET;
using Serilog;

namespace Dalamud.Interface.Internal.Windows.Data.Widgets;

/// <summary>
/// Widget for displaying configuration info.
/// </summary>
internal class VfsWidget : IDataWindowWidget
{
private int numBytes = 1024;
private int reps = 1;

/// <inheritdoc/>
public string[]? CommandShortcuts { get; init; } = { "vfs" };

/// <inheritdoc/>
public string DisplayName { get; init; } = "VFS";

/// <inheritdoc/>
public bool Ready { get; set; }

/// <inheritdoc/>
public void Load()
{
this.Ready = true;
}

/// <inheritdoc/>
public void Draw()
{
var service = Service<ReliableFileStorage>.Get();
var dalamud = Service<Dalamud>.Get();

ImGui.InputInt("Num bytes", ref this.numBytes);
ImGui.InputInt("Reps", ref this.reps);

var path = Path.Combine(dalamud.StartInfo.WorkingDirectory!, "test.bin");

if (ImGui.Button("Write"))
{
Log.Information("=== WRITING ===");
var data = new byte[this.numBytes];
var stopwatch = new Stopwatch();
var acc = 0L;

for (var i = 0; i < this.reps; i++)
{
stopwatch.Restart();
service.WriteAllBytesAsync(path, data).GetAwaiter().GetResult();
stopwatch.Stop();
acc += stopwatch.ElapsedMilliseconds;
Log.Information("Turn {Turn} took {Ms}ms", i, stopwatch.ElapsedMilliseconds);
}

Log.Information("Took {Ms}ms in total", acc);
}

if (ImGui.Button("Read"))
{
Log.Information("=== READING ===");
var stopwatch = new Stopwatch();
var acc = 0L;

for (var i = 0; i < this.reps; i++)
{
stopwatch.Restart();
service.ReadAllBytes(path);
stopwatch.Stop();
acc += stopwatch.ElapsedMilliseconds;
Log.Information("Turn {Turn} took {Ms}ms", i, stopwatch.ElapsedMilliseconds);
}

Log.Information("Took {Ms}ms in total", acc);
}

if (ImGui.Button("Test Config"))
{
var config = Service<DalamudConfiguration>.Get();

Log.Information("=== READING ===");
var stopwatch = new Stopwatch();
var acc = 0L;

for (var i = 0; i < this.reps; i++)
{
stopwatch.Restart();
config.ForceSave();
stopwatch.Stop();
acc += stopwatch.ElapsedMilliseconds;
Log.Information("Turn {Turn} took {Ms}ms", i, stopwatch.ElapsedMilliseconds);
}

Log.Information("Took {Ms}ms in total", acc);
}
}
}
20 changes: 19 additions & 1 deletion Dalamud/Plugin/DalamudPluginInterface.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;

using Dalamud.Configuration;
using Dalamud.Configuration.Internal;
Expand Down Expand Up @@ -338,12 +338,30 @@ public ICallGateSubscriber<T1, T2, T3, T4, T5, T6, T7, T8, TRet> GetIpcSubscribe
/// Save a plugin configuration(inheriting IPluginConfiguration).
/// </summary>
/// <param name="currentConfig">The current configuration.</param>
[Obsolete("Prefer SavePluginConfigAsync() to avoid blocking the main thread.")]
public void SavePluginConfig(IPluginConfiguration? currentConfig)
{
if (currentConfig == null)
return;

#pragma warning disable CS0618 // Type or member is obsolete
this.configs.Save(currentConfig, this.plugin.InternalName, this.plugin.Manifest.WorkingPluginId);
#pragma warning restore CS0618 // Type or member is obsolete
}

/// <summary>
/// Save a plugin configuration(inheriting IPluginConfiguration).
/// </summary>
/// <param name="currentConfig">The current configuration.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task SavePluginConfigAsync(IPluginConfiguration? currentConfig)
{
if (currentConfig == null)
return;

#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
}
Comment on lines +362 to +364
Copy link
Member

Choose a reason for hiding this comment

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

These pragmas are unnecessary, SaveAsync is not obsolete.


/// <summary>
Expand Down
22 changes: 15 additions & 7 deletions Dalamud/Storage/ReliableFileStorage.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.IO;
using System.Text;
using System.Threading.Tasks;

using Dalamud.Logging.Internal;
using Dalamud.Utility;
Expand Down Expand Up @@ -91,8 +92,9 @@ public bool Exists(string path, Guid containerId = default)
/// <param name="path">Path to write to.</param>
/// <param name="contents">The contents of the file.</param>
/// <param name="containerId">Container to write to.</param>
public void WriteAllText(string path, string? contents, Guid containerId = default)
KazWolfe marked this conversation as resolved.
Show resolved Hide resolved
=> this.WriteAllText(path, contents, Encoding.UTF8, containerId);
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task WriteAllTextAsync(string path, string? contents, Guid containerId = default)
=> await this.WriteAllTextAsync(path, contents, Encoding.UTF8, containerId);

/// <summary>
/// Write all text to a file.
Expand All @@ -101,19 +103,21 @@ public void WriteAllText(string path, string? contents, Guid containerId = defau
/// <param name="contents">The contents of the file.</param>
/// <param name="encoding">The encoding to write with.</param>
/// <param name="containerId">Container to write to.</param>
public void WriteAllText(string path, string? contents, Encoding encoding, Guid containerId = default)
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task WriteAllTextAsync(string path, string? contents, Encoding encoding, Guid containerId = default)
{
var bytes = encoding.GetBytes(contents ?? string.Empty);
this.WriteAllBytes(path, bytes, containerId);
await this.WriteAllBytesAsync(path, bytes, containerId);
}

/// <summary>
/// Write all bytes to a file.
/// </summary>
/// <param name="path">Path to write to.</param>
/// <param name="bytes">The contents of the file.</param>
/// <param name="containerId">Container to write to.</param>
public void WriteAllBytes(string path, byte[] bytes, Guid containerId = default)
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public Task WriteAllBytesAsync(string path, byte[] bytes, Guid containerId = default)
{
ArgumentException.ThrowIfNullOrEmpty(path);

Expand All @@ -122,7 +126,7 @@ public void WriteAllBytes(string path, byte[] bytes, Guid containerId = default)
if (this.db == null)
{
Util.WriteAllBytesSafe(path, bytes);
return;
return Task.CompletedTask;
}

this.db.RunInTransaction(() =>
Expand All @@ -148,6 +152,8 @@ public void WriteAllBytes(string path, byte[] bytes, Guid containerId = default)
Util.WriteAllBytesSafe(path, bytes);
});
}

return Task.CompletedTask;
}

/// <summary>
Expand Down Expand Up @@ -258,6 +264,8 @@ public byte[] ReadAllBytes(string path, bool forceBackup = false, Guid container

if (forceBackup)
{
Log.Information("Reading from db");

// If the db failed to load, act as if the file does not exist
if (this.db == null)
throw new FileNotFoundException("Backup database was not available");
Expand Down
Loading