Skip to content

Commit

Permalink
Avoid some unecessary conversions to AbsolutePath
Browse files Browse the repository at this point in the history
  • Loading branch information
@dfederm
dfederm committed Jan 20, 2024
1 parent 71de336 commit 32fe4b6
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 64 deletions.
3 changes: 1 addition & 2 deletions src/AzureBlobStorage/MSBuildCacheAzureBlobStoragePlugin.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using BuildXL.Cache.ContentStore.Distributed.NuCache;
using BuildXL.Cache.ContentStore.Hashing;
using BuildXL.Cache.ContentStore.Interfaces.Auth;
using BuildXL.Cache.ContentStore.Interfaces.FileSystem;
using BuildXL.Cache.ContentStore.Interfaces.Results;
using BuildXL.Cache.ContentStore.Interfaces.Stores;
using BuildXL.Cache.ContentStore.Interfaces.Tracing;
Expand Down Expand Up @@ -91,7 +90,7 @@ protected override async Task<ICacheClient> CreateCacheClientAsync(PluginLoggerB
localCacheSession,
(remoteCache, remoteCacheSession, twoLevelConfig),
ContentHasher,
new AbsolutePath(Settings.RepoRoot),
Settings.RepoRoot,
NodeContextRepository,
GetFileRealizationMode,
Settings.MaxConcurrentCacheContentOperations,
Expand Down
3 changes: 1 addition & 2 deletions src/AzurePipelines/MSBuildCacheAzurePipelinesPlugin.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Threading.Tasks;
using BuildXL.Cache.ContentStore.Distributed.NuCache;
using BuildXL.Cache.ContentStore.Hashing;
using BuildXL.Cache.ContentStore.Interfaces.FileSystem;
using BuildXL.Cache.ContentStore.Interfaces.Results;
using BuildXL.Cache.ContentStore.Interfaces.Stores;
using BuildXL.Cache.ContentStore.Interfaces.Tracing;
Expand Down Expand Up @@ -56,7 +55,7 @@ protected override async Task<ICacheClient> CreateCacheClientAsync(PluginLoggerB
localCacheSession,
cacheLogger,
Settings.CacheUniverse,
new AbsolutePath(Settings.RepoRoot),
Settings.RepoRoot,
NodeContextRepository,
GetFileRealizationMode,
Settings.MaxConcurrentCacheContentOperations,
Expand Down
62 changes: 31 additions & 31 deletions src/AzurePipelines/PipelineCachingCacheClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ namespace Microsoft.MSBuildCache.AzurePipelines;
internal sealed class PipelineCachingCacheClient : CacheClient
#pragma warning restore CS1570 // XML comment has badly formed XML
{
////private readonly record struct ContentHashWithPath(ContentHash Hash, string Path);

private const string InternalMetadataPathPrefix = "/???";

private const string NodeBuildResultRelativePath = $"{InternalMetadataPathPrefix}/NodeBuildResult";
Expand Down Expand Up @@ -93,7 +95,7 @@ public PipelineCachingCacheClient(
IContentSession localCAS,
ILogger logger,
string universe,
AbsolutePath repoRoot,
string repoRoot,
INodeContextRepository nodeContextRepository,
Func<string, FileRealizationMode> getFileRealizationMode,
int maxConcurrentCacheContentOperations,
Expand Down Expand Up @@ -163,7 +165,7 @@ public PipelineCachingCacheClient(
protected override async Task AddNodeAsync(
Context context,
StrongFingerprint fingerprint,
IReadOnlyDictionary<AbsolutePath, ContentHash> outputs,
IReadOnlyDictionary<string, ContentHash> outputs,
(ContentHash hash, byte[] bytes) nodeBuildResultBytes,
(ContentHash hash, byte[] bytes)? pathSetBytes,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -208,39 +210,39 @@ protected override async Task AddNodeAsync(

// 2. Link out unique content to the temp folder

Dictionary<ContentHash, AbsolutePath> tempFilesPerHash = outputs.Values.Distinct().ToDictionary(
Dictionary<ContentHash, string> tempFilesPerHash = outputs.Values.Distinct().ToDictionary(
hash => hash,
hash =>
{
string tempFilePath = Path.Combine(TempFolder, Guid.NewGuid().ToString("N") + ".tmp");
tempFilePaths.Add(tempFilePath);
return new AbsolutePath(tempFilePath);
return tempFilePath;
});

List<ContentHashWithPath> tempFiles = tempFilesPerHash
.Select(kvp => new ContentHashWithPath(kvp.Key, kvp.Value))
.Select(kvp => new ContentHashWithPath(kvp.Key, new AbsolutePath(kvp.Value)))
.ToList();

Dictionary<AbsolutePath, PlaceFileResult> placeResults = await TryPlaceFilesFromCacheAsync(context, tempFiles, cancellationToken);
Dictionary<string, PlaceFileResult> placeResults = await TryPlaceFilesFromCacheAsync(context, tempFiles, cancellationToken);
foreach (PlaceFileResult placeResult in placeResults.Values)
{
placeResult.ThrowIfFailure();
}

// 3. map all the relative paths to the temp files
foreach (KeyValuePair<AbsolutePath, ContentHash> output in outputs)
foreach (KeyValuePair<string, ContentHash> output in outputs)
{
string relativePath = output.Key.Path.Replace(RepoRoot.Path, "", StringComparison.OrdinalIgnoreCase);
extras.Add(relativePath.Replace("\\", "/", StringComparison.Ordinal), new FileInfo(tempFilesPerHash[output.Value].Path));
string relativePath = output.Key.MakePathRelativeTo(RepoRoot)!;
extras.Add(relativePath.Replace("\\", "/", StringComparison.Ordinal), new FileInfo(tempFilesPerHash[output.Value]));
}
}
else
{
infos = outputs.Keys.Select(f => new FileInfo(f.Path)).ToArray();
infos = outputs.Keys.Select(f => new FileInfo(f)).ToArray();
}

var result = await WithHttpRetries(
() => _manifestClient.PublishAsync(RepoRoot.Path, infos, extras, new ArtifactPublishOptions(), manifestFileOutputPath: null, cancellationToken),
() => _manifestClient.PublishAsync(RepoRoot, infos, extras, new ArtifactPublishOptions(), manifestFileOutputPath: null, cancellationToken),
context: $"Publishing content for {fingerprint}",
cancellationToken);

Expand Down Expand Up @@ -371,15 +373,15 @@ private static byte GetAlgorithmId(ContentHash hash)
}
}

private async Task<Dictionary<AbsolutePath, PlaceFileResult>> TryPlaceFilesFromCacheAsync(Context context, IReadOnlyList<ContentHashWithPath> files, CancellationToken cancellationToken)
private async Task<Dictionary<string, PlaceFileResult>> TryPlaceFilesFromCacheAsync(Context context, List<ContentHashWithPath> files, CancellationToken cancellationToken)
{
// cache expects destination directories already exist
foreach (ContentHashWithPath file in files)
{
CreateParentDirectory(file.Path);
CreateParentDirectory(file.Path.Path);
}

Dictionary<AbsolutePath, PlaceFileResult> results = new();
Dictionary<string, PlaceFileResult> results = new();
List<ContentHashWithPath> places = new();

foreach (IGrouping<(byte algoId, FileRealizationMode mode), ContentHashWithPath>? filesGroup in files.GroupBy(f => (GetAlgorithmId(f.Hash), GetFileRealizationMode(f.Path.Path))))
Expand Down Expand Up @@ -415,7 +417,7 @@ private async Task<Dictionary<AbsolutePath, PlaceFileResult>> TryPlaceFilesFromC
foreach (Task<Indexed<PlaceFileResult>> resultTask in groupResults)
{
Indexed<PlaceFileResult> result = await resultTask;
results.Add(places[result.Index].Path, result.Item);
results.Add(places[result.Index].Path.Path, result.Item);
}
}

Expand Down Expand Up @@ -471,7 +473,7 @@ public void Dispose() { }
public Task<Stream?> GetNodeBuildResultAsync(Context context, CancellationToken cancellationToken) =>
Task.FromResult((Stream?)new MemoryStream(_nodeBuildResultBytes));

public async Task PlaceFilesAsync(Context context, IReadOnlyDictionary<AbsolutePath, ContentHash> files, CancellationToken cancellationToken)
public async Task PlaceFilesAsync(Context context, IReadOnlyDictionary<string, ContentHash> files, CancellationToken cancellationToken)
{
_client.Tracer.Debug(context, $"Placing manifest `{_manifestId}`.");

Expand All @@ -480,18 +482,18 @@ public async Task PlaceFilesAsync(Context context, IReadOnlyDictionary<AbsoluteP
ThrowIfDifferent(manifestFiles, requestFiles, $"Manifest `{_manifestId}` and PlaceFiles don't match:");

// try to pull whole files from the cache
var places = files.Select(f => new ContentHashWithPath(f.Value, f.Key)).ToList();
var places = files.Select(f => new ContentHashWithPath(f.Value, new AbsolutePath(f.Key))).ToList();

Dictionary<AbsolutePath, PlaceFileResult> placeResults = await _client.TryPlaceFilesFromCacheAsync(context, places, cancellationToken);
Dictionary<string, PlaceFileResult> placeResults = await _client.TryPlaceFilesFromCacheAsync(context, places, cancellationToken);

Dictionary<AbsolutePath, ManifestItem> manifestItems = _manifest.Items.ToDictionary(i => _client.RepoRoot / new RelativePath(i.Path), i => i);
Dictionary<string, ManifestItem> manifestItems = _manifest.Items.ToDictionary(i => Path.Combine(_client.RepoRoot, i.Path), i => i);
var itemsToDownload = new List<ManifestItem>();
var toAddToCacheAsWholeFile = new Dictionary<ContentHash, AbsolutePath>();
foreach (KeyValuePair<AbsolutePath, PlaceFileResult> placeResult in placeResults)
var toAddToCacheAsWholeFile = new Dictionary<ContentHash, string>();
foreach (KeyValuePair<string, PlaceFileResult> placeResult in placeResults)
{
if (!placeResult.Value.Succeeded)
{
AbsolutePath path = placeResult.Key;
string path = placeResult.Key;
itemsToDownload.Add(manifestItems[path]);

ContentHash hash = files[path];
Expand All @@ -517,9 +519,7 @@ public async Task PlaceFilesAsync(Context context, IReadOnlyDictionary<AbsoluteP
await File.WriteAllTextAsync(tempManifestFile.Path, JsonSerializer.Serialize(tempManifest), cancellationToken);
#endif

var manifestOptions = DownloadDedupManifestArtifactOptions.CreateWithManifestPath(
tempManifestFile.Path,
_client.RepoRoot.Path);
var manifestOptions = DownloadDedupManifestArtifactOptions.CreateWithManifestPath(tempManifestFile.Path, _client.RepoRoot);

await _client.WithHttpRetries(
async () =>
Expand All @@ -530,11 +530,11 @@ await _client.WithHttpRetries(
context: context.ToString()!,
cancellationToken);

foreach (KeyValuePair<ContentHash, AbsolutePath> addToCache in toAddToCacheAsWholeFile)
foreach (KeyValuePair<ContentHash, string> addToCache in toAddToCacheAsWholeFile)
{
ContentHash hash = addToCache.Key;
AbsolutePath path = addToCache.Value;
await _client.LocalCacheSession.PutFileAsync(context, hash, path, _client.GetFileRealizationMode(path.Path), cancellationToken);
string path = addToCache.Value;
await _client.LocalCacheSession.PutFileAsync(context, hash, new AbsolutePath(path), _client.GetFileRealizationMode(path), cancellationToken);
}
}
}
Expand Down Expand Up @@ -610,13 +610,13 @@ private static SortedDictionary<RelativePath, DedupIdentifier> CreateNormalizedM
return sorted;
}

private SortedDictionary<RelativePath, DedupIdentifier> CreateNormalizedManifest(IReadOnlyDictionary<AbsolutePath, ContentHash> files)
private SortedDictionary<RelativePath, DedupIdentifier> CreateNormalizedManifest(IReadOnlyDictionary<string, ContentHash> files)
{
SortedDictionary<RelativePath, DedupIdentifier> sorted = new(RelativePathComparer.Instance);

foreach (KeyValuePair<AbsolutePath, ContentHash> f in files)
foreach (KeyValuePair<string, ContentHash> f in files)
{
sorted.Add(new RelativePath(f.Key.Path.Replace(RepoRoot.Path, "", StringComparison.OrdinalIgnoreCase)), f.Value.ToBlobIdentifier().ToDedupIdentifier());
sorted.Add(new RelativePath(f.Key.MakePathRelativeTo(RepoRoot)!), f.Value.ToBlobIdentifier().ToDedupIdentifier());
}

return sorted;
Expand Down
22 changes: 11 additions & 11 deletions src/Common/Caching/CacheClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public abstract class CacheClient : ICacheClient
private readonly OutputHasher _outputHasher;
private readonly ConcurrentDictionary<NodeContext, Task> _publishingTasks = new();
private readonly ConcurrentDictionary<NodeContext, Task> _materializationTasks = new();
private readonly ConcurrentDictionary<AbsolutePath, bool> _directoryCreationCache = new();
private readonly ConcurrentDictionary<string, bool> _directoryCreationCache = new();
private readonly IContentHasher _hasher;
private readonly IFingerprintFactory _fingerprintFactory;
private readonly INodeContextRepository _nodeContextRepository;
Expand All @@ -44,7 +44,7 @@ protected CacheClient(
Context rootContext,
IFingerprintFactory fingerprintFactory,
IContentHasher hasher,
AbsolutePath repoRoot,
string repoRoot,
INodeContextRepository nodeContextRepository,
Func<string, FileRealizationMode> getFileRealizationMode,
ICache localCache,
Expand Down Expand Up @@ -89,7 +89,7 @@ protected CacheClient(

protected Context RootContext { get; }

protected AbsolutePath RepoRoot { get; }
protected string RepoRoot { get; }

protected Selector EmptySelector { get; }

Expand All @@ -109,7 +109,7 @@ protected CacheClient(
protected abstract Task AddNodeAsync(
Context context,
StrongFingerprint fingerprint,
IReadOnlyDictionary<AbsolutePath, ContentHash> outputs,
IReadOnlyDictionary<string, ContentHash> outputs,
(ContentHash hash, byte[] bytes) nodeBuildResultBytes,
(ContentHash hash, byte[] bytes)? pathSetBytes,
CancellationToken cancellationToken);
Expand All @@ -127,7 +127,7 @@ protected abstract IAsyncEnumerable<Selector> GetSelectors(
protected interface ICacheEntry : IDisposable
{
Task<Stream?> GetNodeBuildResultAsync(Context context, CancellationToken cancellationToken);
Task PlaceFilesAsync(Context context, IReadOnlyDictionary<AbsolutePath, ContentHash> files, CancellationToken cancellationToken);
Task PlaceFilesAsync(Context context, IReadOnlyDictionary<string, ContentHash> files, CancellationToken cancellationToken);
}

protected async Task ShutdownCacheAsync(ICache cache)
Expand Down Expand Up @@ -189,16 +189,16 @@ async Task DrainTasksAsync(ConcurrentDictionary<NodeContext, Task> tasks, string
}
}

protected void CreateParentDirectory(AbsolutePath filePath)
protected void CreateParentDirectory(string filePath)
{
AbsolutePath? parentDirectory = filePath.Parent;
string? parentDirectory = Path.GetDirectoryName(filePath);
if (parentDirectory is not null)
{
_directoryCreationCache.GetOrAdd(
parentDirectory,
dir =>
{
Directory.CreateDirectory(dir.Path);
Directory.CreateDirectory(dir);
return true;
});
}
Expand Down Expand Up @@ -299,8 +299,8 @@ public async Task AddNodeInternalAsync(
pathSetBytes = null;
}

Dictionary<AbsolutePath, ContentHash> outputs = nodeBuildResult.Outputs.ToDictionary(
kvp => RepoRoot / kvp.Key,
Dictionary<string, ContentHash> outputs = nodeBuildResult.Outputs.ToDictionary(
kvp => Path.Combine(RepoRoot, kvp.Key),
kvp => kvp.Value);

Fingerprint? weakFingerprint = _fingerprintFactory.GetWeakFingerprint(nodeContext);
Expand Down Expand Up @@ -396,7 +396,7 @@ await AddNodeAsync(

Func<CancellationToken, Task> placeFilesAsync = (ct) => cacheEntry.PlaceFilesAsync(
context,
nodeBuildResult.Outputs.ToDictionary(o => RepoRoot / o.Key, o => o.Value),
nodeBuildResult.Outputs.ToDictionary(o => Path.Combine(RepoRoot, o.Key), o => o.Value),
ct);

if (_enableAsyncMaterialization)
Expand Down
Loading

0 comments on commit 32fe4b6

Please sign in to comment.