Skip to content

Commit

Permalink
Make Mod Component resolution async.
Browse files Browse the repository at this point in the history 
Although using `Parallel` here made some sense due to the operation being relatively slow, it was not a good choice because all of the blocking I/O was causing thread pool exhaustion on startup. This led to some slowdowns or even apparent deadlocking especially when debugging.

Resolution is now async so that the I/O can be async. This allows much more real work to be done in the same amount of time, and avoids tying up all the threads.

#10
  • Loading branch information
@focustense
focustense committed Aug 11, 2021
1 parent acfdc52 commit 71fcfb3
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 39 deletions.
24 changes: 12 additions & 12 deletions Focus.ModManagers.ModOrganizer.Tests/ModOrganizerComponentResolverTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ public ModOrganizerComponentResolverTests()
}

[Fact]
public void WhenMetaIniNotFound_ResolvesDefaultComponent()
public async Task WhenMetaIniNotFound_ResolvesDefaultComponent()
{
var component = componentResolver.ResolveComponentInfo("foo");
var component = await componentResolver.ResolveComponentInfo("foo");

Assert.Equal("foo", component.Id);
Assert.Equal("foo", component.Name);
Expand All @@ -38,10 +38,10 @@ public void WhenMetaIniNotFound_ResolvesDefaultComponent()
}

[Fact]
public void WhenMetaIniIsEmpty_ResolvesDefaultComponent()
public async Task WhenMetaIniIsEmpty_ResolvesDefaultComponent()
{
fs.AddFile(@$"{RootPath}\foo\meta.ini", new MockFileData(""));
var component = componentResolver.ResolveComponentInfo("foo");
var component = await componentResolver.ResolveComponentInfo("foo");

Assert.Equal("foo", component.Id);
Assert.Equal("foo", component.Name);
Expand All @@ -51,7 +51,7 @@ public void WhenMetaIniIsEmpty_ResolvesDefaultComponent()
}

[Fact]
public void WhenMetaProvidesModInfo_AndNoDownloadAvailable_ReturnsDefaultWithId()
public async Task WhenMetaProvidesModInfo_AndNoDownloadAvailable_ReturnsDefaultWithId()
{
var ini = new IniData();
ini.AddSection("General", new()
Expand All @@ -64,7 +64,7 @@ public void WhenMetaProvidesModInfo_AndNoDownloadAvailable_ReturnsDefaultWithId(
{ @"1\fileid", "12345" },
});
fs.AddFile(@$"{RootPath}\foo\meta.ini", new MockFileData(ini.ToString()));
var component = componentResolver.ResolveComponentInfo("foo");
var component = await componentResolver.ResolveComponentInfo("foo");

Assert.Equal("12345", component.Id);
Assert.Equal("foo", component.Name);
Expand All @@ -74,7 +74,7 @@ public void WhenMetaProvidesModInfo_AndNoDownloadAvailable_ReturnsDefaultWithId(
}

[Fact]
public void WhenMetaProvidesModInfo_AndLinksToValidDownload_ReturnsFullModInfo()
public async Task WhenMetaProvidesModInfo_AndLinksToValidDownload_ReturnsFullModInfo()
{
const string downloadDirectory = @"D:\Mod Organizer Downloads";
configurationMock.SetupGet(x => x.DownloadDirectory).Returns(downloadDirectory);
Expand All @@ -92,7 +92,7 @@ public void WhenMetaProvidesModInfo_AndLinksToValidDownload_ReturnsFullModInfo()
{ "modName", "My Awesome Mod" },
});
fs.AddFile($@"{downloadDirectory}\foo_file.7z.meta", downloadIni.ToString());
var component = componentResolver.ResolveComponentInfo("foo");
var component = await componentResolver.ResolveComponentInfo("foo");

Assert.Equal("8372", component.Id);
Assert.Equal("foo", component.Name);
Expand All @@ -102,20 +102,20 @@ public void WhenMetaProvidesModInfo_AndLinksToValidDownload_ReturnsFullModInfo()
}

[Fact]
public void WhenComponentNameIsBackup_AndMetaIniNotFound_ReturnsDisabledComponent()
public async Task WhenComponentNameIsBackup_AndMetaIniNotFound_ReturnsDisabledComponent()
{
var component = componentResolver.ResolveComponentInfo("foo_backup4");
var component = await componentResolver.ResolveComponentInfo("foo_backup4");

Assert.False(component.IsEnabled);
}

[Fact]
public void WhenComponentNameIsBackup_AndMetaProvidesModInfo_ReturnsDisabledComponent()
public async Task WhenComponentNameIsBackup_AndMetaProvidesModInfo_ReturnsDisabledComponent()
{
var ini = new IniData();
ini.AddSection("General", new() { { "modid", "17436" } });
fs.AddFile(@$"{RootPath}\foo\meta.ini", new MockFileData(ini.ToString()));
var component = componentResolver.ResolveComponentInfo("foo_backup26");
var component = await componentResolver.ResolveComponentInfo("foo_backup26");

Assert.False(component.IsEnabled);
}
Expand Down
16 changes: 11 additions & 5 deletions Focus.ModManagers.ModOrganizer/ModOrganizerComponentResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.IO;
using System.IO.Abstractions;
using System.Text.RegularExpressions;
using System.Threading.Tasks;

namespace Focus.ModManagers.ModOrganizer
{
Expand All @@ -29,7 +30,7 @@ public ModOrganizerComponentResolver(IFileSystem fs, IModOrganizerConfiguration
this.rootPath = rootPath;
}

public ModComponentInfo ResolveComponentInfo(string componentName)
public async Task<ModComponentInfo> ResolveComponentInfo(string componentName)
{
// TODO: To actually know if a mod is enabled, we would need to first determine the current profile and then
// look at that profile's INI. For the moment, this is just a quick fix to prevent backups from appearing.
Expand All @@ -46,7 +47,7 @@ public ModComponentInfo ResolveComponentInfo(string componentName)
var installationFile = string.Empty;
try
{
var metaIni = ReadIniFile(metaPath);
var metaIni = await ReadIniFile(metaPath);
modId = metaIni?["General"]?["modid"] ?? string.Empty;
installationFile = metaIni?["General"]?["installationFile"] ?? string.Empty;
fileId = metaIni?["installedFiles"]?[@"1\fileid"] ?? string.Empty;
Expand All @@ -60,7 +61,7 @@ public ModComponentInfo ResolveComponentInfo(string componentName)
try
{
var downloadMetaPath = fs.Path.Combine(config.DownloadDirectory, $"{installationFile}.meta");
var downloadIni = ReadIniFile(downloadMetaPath);
var downloadIni = await ReadIniFile(downloadMetaPath);
if (string.IsNullOrEmpty(fileId))
fileId = downloadIni?["General"]["fileID"] ?? string.Empty;
modName = downloadIni?["General"]["modName"] ?? string.Empty;
Expand All @@ -79,11 +80,16 @@ public ModComponentInfo ResolveComponentInfo(string componentName)
return new ModComponentInfo(key, componentId, componentName, modPath, isEnabled);
}

private IniData ReadIniFile(string fileName)
private async Task<IniData> ReadIniFile(string fileName)
{
if (!fs.File.Exists(fileName))
return new IniData();
using var stream = fs.File.OpenRead(fileName);
// The IniParser doesn't directly support async I/O, but since the resolver is going to be processing a
// whole lot of files, it's very important that we don't depend on blocking I/O.
// Since the INI files are quite small, one hacky solution is to copy their contents (asynchronously) into
// a memory stream, and then let the INI parser handle that.
var bytes = await fs.File.ReadAllBytesAsync(fileName);
using var stream = new MemoryStream(bytes);
using var reader = new StreamReader(stream);
return IniParser.ReadData(reader);
}
Expand Down
7 changes: 3 additions & 4 deletions Focus.ModManagers.Tests/IndexedModRepositoryTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
using Focus.Testing.Files;
using Moq;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Xunit;

namespace Focus.ModManagers.Tests
Expand Down Expand Up @@ -31,14 +30,14 @@ public IndexedModRepositoryTests()
archiveProvider.AddFiles(@$"{RootPath}\01_bar\bar.bsa", "bar3", "common/c1");
componentResolverMock = new Mock<IComponentResolver>();
componentResolverMock.Setup(x => x.ResolveComponentInfo(It.IsAny<string>()))
.Returns((string componentName) => new ModComponentInfo(
.Returns((string componentName) => Task.FromResult(new ModComponentInfo(
componentName.IndexOf('_') == 2 ?
new ModLocatorKey(componentName[0..2], modNames[componentName[0..2]]) :
ModLocatorKey.Empty,
componentName.IndexOf('_') == 2 ? $"{componentName[3..]}_id" : $"{componentName}_id",
componentName.IndexOf('_') == 2 ? componentName[3..] : componentName,
Path.Combine(RootPath, componentName),
!componentName.EndsWith("_disabled")));
!componentName.EndsWith("_disabled"))));
modNames = new()
{
{ "01", "modname1" },
Expand Down
6 changes: 4 additions & 2 deletions Focus.ModManagers/IComponentResolver.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
namespace Focus.ModManagers
using System.Threading.Tasks;

namespace Focus.ModManagers
{
public interface IComponentResolver
{
ModComponentInfo ResolveComponentInfo(string componentName);
Task<ModComponentInfo> ResolveComponentInfo(string componentName);
}
}
28 changes: 14 additions & 14 deletions Focus.ModManagers/IndexedModRepository.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;

namespace Focus.ModManagers
{
public class IndexedModRepository : IModRepository
{
record ResolvedBucket(string BucketName, ModComponentInfo Component);

// For all names used here: Bucket names, component names, mod names.
private static readonly StringComparer NameComparer = StringComparer.CurrentCultureIgnoreCase;

Expand Down Expand Up @@ -45,19 +48,8 @@ public IndexedModRepository(
this.modIndex = modIndex;
this.rootPath = rootPath;

// In theory, we don't need the temporary list - it's really just a waste of CPU and memory to materialize
// eagerly before adding to the following dictionary.
// However, there doesn't seem to be any other way to maintain a stable ordering as it gets added to the
// dictionary. Simply using AsOrdered will destabilize the tests, specifically the ones that verify behavior
// of mod name/key duplication.
// There might be a better way - but most of the real latency is in the resolver anyway, so despite being a
// little wasteful, it's mostly invisible.
var bucketComponentList = modIndex.GetBucketNames()
.AsParallel().AsOrdered()
.Select(b => new { BucketName = b, Component = componentResolver.ResolveComponentInfo(b) })
.ToList();
bucketNamesToComponents =
bucketComponentList.ToDictionary(x => x.BucketName, x => x.Component, NameComparer);
var bucketComponents = ResolveAllBuckets().Result;
bucketNamesToComponents = bucketComponents.ToDictionary(x => x.BucketName, x => x.Component, NameComparer);
componentNamesToBucketNames =
bucketNamesToComponents.ToDictionary(x => x.Value.Name, x => x.Key, NameComparer);
componentNamesToComponents = bucketNamesToComponents.Values.ToDictionary(x => x.Name, NameComparer);
Expand Down Expand Up @@ -238,6 +230,14 @@ private void RegisterComponent(ModComponentInfo component, string bucketName)
}
}

private async Task<IEnumerable<ResolvedBucket>> ResolveAllBuckets()
{
var resolveTasks = modIndex.GetBucketNames()
.Select(async b => new ResolvedBucket(b, await componentResolver.ResolveComponentInfo(b)));
var resolved = await Task.WhenAll(resolveTasks);
return resolved.NotNull();
}

private void SetupArchiveIndex()
{
var archivePathPairs = modIndex.GetBucketedFilePaths()
Expand Down Expand Up @@ -268,7 +268,7 @@ private void SetupEvents()

if (!bucketNamesToComponents.ContainsKey(e.BucketName))
{
var component = componentResolver.ResolveComponentInfo(e.BucketName);
var component = componentResolver.ResolveComponentInfo(e.BucketName).Result;
RegisterComponent(component, e.BucketName);
}
};
Expand Down
4 changes: 2 additions & 2 deletions Focus.ModManagers/ModLocatorKey.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ public override string ToString()
return prefix + string.Join(separator, Id, Name);
}

public static bool operator ==(ModLocatorKey x, IModLocatorKey y)
public static bool operator ==(ModLocatorKey? x, IModLocatorKey? y)
{
return Equals(x, y);
}

public static bool operator !=(ModLocatorKey x, IModLocatorKey y)
public static bool operator !=(ModLocatorKey? x, IModLocatorKey? y)
{
return !(x == y);
}
Expand Down

0 comments on commit 71fcfb3

Please sign in to comment.