Skip to content

[StaticWebAssets] Cleanup ApplyCompressionNegotiation.cs and ResolveCompressedAssets.cs #48082

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.

Sign up for GitHub

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

Merged
javiercn merged 5 commits into from
Apr 22, 2025
Merged

[StaticWebAssets] Cleanup ApplyCompressionNegotiation.cs and ResolveCompressedAssets.cs #48082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f323627
tmp
javiercn Apr 1, 2025
c9183e8
Reduce allocations
javiercn Apr 1, 2025
6125633
More allocation cleanups
javiercn Apr 1, 2025
d8fbcd1
Update test assertions
javiercn Apr 4, 2025
829e3e3
Update src/StaticWebAssetsSdk/Tasks/ApplyCompressionNegotiation.cs
javiercn Apr 22, 2025
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
270 changes: 186 additions & 84 deletions src/StaticWebAssetsSdk/Tasks/ApplyCompressionNegotiation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,59 +23,35 @@ public override bool Execute()
{
var assetsById = StaticWebAsset.ToAssetDictionary(CandidateAssets);

var endpointsByAsset = CandidateEndpoints.Select(StaticWebAssetEndpoint.FromTaskItem)
.GroupBy(e => e.AssetFile)
.ToDictionary(g => g.Key, g => g.ToList());
var endpointsByAsset = StaticWebAssetEndpoint.ToAssetFileDictionary(CandidateEndpoints);

var compressedAssets = assetsById.Values.Where(a => a.AssetTraitName == "Content-Encoding").ToList();
var updatedEndpoints = new HashSet<StaticWebAssetEndpoint>(StaticWebAssetEndpoint.RouteAndAssetComparer);
var updatedEndpoints = new HashSet<StaticWebAssetEndpoint>(CandidateEndpoints.Length, StaticWebAssetEndpoint.RouteAndAssetComparer);

var preservedEndpoints = new Dictionary<(string, string), StaticWebAssetEndpoint>();
var compressionHeadersByEncoding = new Dictionary<string, StaticWebAssetEndpointResponseHeader[]>(2);
Copy link
Member

@MackinnonBuck MackinnonBuck Apr 21, 2025

Choose a reason for hiding this comment

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

Is 2 the expected maximum capacity? If so, might a dictionary be overkill instead of, e.g., a fixed-size buffer and linear search?

Copy link
Member Author

@javiercn javiercn Apr 22, 2025

Choose a reason for hiding this comment

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

That's fair, but the main goal here was to remove the allocations. We can do a further pass in the future if needed


// Add response headers to compressed endpoints
foreach (var compressedAsset in compressedAssets)
foreach (var compressedAsset in assetsById.Values)
{
if (!assetsById.TryGetValue(compressedAsset.RelatedAsset, out var relatedAsset))
if (!string.Equals(compressedAsset.AssetTraitName, "Content-Encoding", StringComparison.Ordinal))
{
Log.LogWarning("Related asset not found for compressed asset: {0}", compressedAsset.Identity);
throw new InvalidOperationException($"Related asset not found for compressed asset: {compressedAsset.Identity}");
}

if (!endpointsByAsset.TryGetValue(compressedAsset.Identity, out var compressedEndpoints))
{
Log.LogWarning("Endpoints not found for compressed asset: {0} {1}", compressedAsset.RelativePath, compressedAsset.Identity);
throw new InvalidOperationException($"Endpoints not found for compressed asset: {compressedAsset.Identity}");
continue;
}

if (!endpointsByAsset.TryGetValue(relatedAsset.Identity, out var relatedAssetEndpoints))
{
Log.LogWarning("Endpoints not found for related asset: {0}", relatedAsset.Identity);
throw new InvalidOperationException($"Endpoints not found for related asset: {relatedAsset.Identity}");
}
var (compressedEndpoints, relatedAssetEndpoints) = ResolveEndpoints(assetsById, endpointsByAsset, compressedAsset);

Log.LogMessage("Processing compressed asset: {0}", compressedAsset.Identity);
StaticWebAssetEndpointResponseHeader[] compressionHeaders = [
new()
{
Name = "Content-Encoding",
Value = compressedAsset.AssetTraitValue
},
new()
{
Name = "Vary",
Value = "Content-Encoding"
}
];
var compressionHeaders = GetOrCreateCompressionHeaders(compressionHeadersByEncoding, compressedAsset);

var quality = ResolveQuality(compressedAsset);
foreach (var compressedEndpoint in compressedEndpoints)
{
if (compressedEndpoint.Selectors.Any(s => string.Equals(s.Name, "Content-Encoding", StringComparison.Ordinal)))
if (HasContentEncodingSelector(compressedEndpoint))
{
Log.LogMessage(MessageImportance.Low, $" Skipping endpoint '{compressedEndpoint.Route}' since it already has a Content-Encoding selector");
Log.LogMessage(MessageImportance.Low, " Skipping endpoint '{0}' since it already has a Content-Encoding selector", compressedEndpoint.Route);
continue;
}
if (!compressedEndpoint.ResponseHeaders.Any(s => string.Equals(s.Name, "Content-Encoding", StringComparison.Ordinal)))

if (!HasContentEncodingResponseHeader(compressedEndpoint))
{
// Add the Content-Encoding and Vary headers
compressedEndpoint.ResponseHeaders = [
Expand All @@ -84,6 +60,8 @@ public override bool Execute()
];
}

var compressedHeaders = GetCompressedHeaders(compressedEndpoint);

Log.LogMessage(MessageImportance.Low, " Updated endpoint '{0}' with Content-Encoding and Vary headers", compressedEndpoint.Route);
updatedEndpoints.Add(compressedEndpoint);

Expand All @@ -93,55 +71,18 @@ public override bool Execute()
{
continue;
}
Log.LogMessage(MessageImportance.Low, "Processing related endpoint '{0}'", relatedEndpointCandidate.Route);
var encodingSelector = new StaticWebAssetEndpointSelector
{
Name = "Content-Encoding",
Value = compressedAsset.AssetTraitValue,
Quality = quality
};
Log.LogMessage(MessageImportance.Low, " Created Content-Encoding selector for compressed asset '{0}' with size '{1}' is '{2}'", encodingSelector.Value, encodingSelector.Quality, relatedEndpointCandidate.Route);
var endpointCopy = new StaticWebAssetEndpoint
{
AssetFile = compressedAsset.Identity,
Route = relatedEndpointCandidate.Route,
Selectors = [
..relatedEndpointCandidate.Selectors,
encodingSelector
],
EndpointProperties = [.. relatedEndpointCandidate.EndpointProperties]
};

var headers = new List<StaticWebAssetEndpointResponseHeader>();
var compressedHeaders = new HashSet<string>(compressedEndpoint.ResponseHeaders.Select(h => h.Name), StringComparer.Ordinal);
ApplyCompressedEndpointHeaders(headers, compressedEndpoint, relatedEndpointCandidate.Route);
ApplyRelatedEndpointCandidateHeaders(headers, relatedEndpointCandidate, compressedHeaders);
endpointCopy.ResponseHeaders = [.. headers];

// Update the endpoint
Log.LogMessage(MessageImportance.Low, " Updated related endpoint '{0}' with Content-Encoding selector '{1}={2}'", relatedEndpointCandidate.Route, encodingSelector.Value, encodingSelector.Quality);
updatedEndpoints.Add(endpointCopy);

var endpointCopy = CreateUpdatedEndpoint(compressedAsset, quality, compressedEndpoint, compressedHeaders, relatedEndpointCandidate);
updatedEndpoints.Add(endpointCopy);
// Since we are going to remove the endpoints from the associated item group and the route is
// the ItemSpec, we want to add the original as well so that it gets re-added.
// The endpoint pointing to the uncompressed asset doesn't have a Content-Encoding selector and
// will use the default "identity" encoding during content negotiation.
if (!preservedEndpoints.ContainsKey((relatedEndpointCandidate.Route, relatedEndpointCandidate.AssetFile)))
{
preservedEndpoints.Add(
(relatedEndpointCandidate.Route, relatedEndpointCandidate.AssetFile),
relatedEndpointCandidate);
}
updatedEndpoints.Add(relatedEndpointCandidate);
}
}
}

// Add the preserved endpoints to the list of updated endpoints.
foreach (var preservedEndpoint in preservedEndpoints.Values)
{
updatedEndpoints.Add(preservedEndpoint);
}

// Before we return the updated endpoints we need to capture any other endpoint whose asset is not associated
// with the compressed asset. This is because we are going to remove the endpoints from the associated item group
// and the route is the ItemSpec, so it will cause those endpoints to be removed.
Expand Down Expand Up @@ -169,11 +110,11 @@ public override bool Execute()
// We now have only endpoints that might have the same route but point to different assets
// and we want to include them in the updated endpoints so that we don't incorrectly remove
// them from the associated item group when we update the endpoints.
var endpointsByRoute = endpointsByAsset.Values.SelectMany(e => e).GroupBy(e => e.Route).ToDictionary(g => g.Key, g => g.ToList());

var updatedEndpointsByRoute = updatedEndpoints.Select(e => e.Route).ToArray();
foreach (var route in updatedEndpointsByRoute)
var endpointsByRoute = GetEndpointsByRoute(endpointsByAsset);
var additionalUpdatedEndpoints = new HashSet<StaticWebAssetEndpoint>(updatedEndpoints.Count, StaticWebAssetEndpoint.RouteAndAssetComparer);
foreach (var updatedEndpoint in updatedEndpoints)
{
var route = updatedEndpoint.Route;
Log.LogMessage(MessageImportance.Low, "Processing route '{0}'", route);
if (endpointsByRoute.TryGetValue(route, out var endpoints))
{
Expand All @@ -184,26 +125,187 @@ public override bool Execute()
}
foreach (var endpoint in endpoints)
{
updatedEndpoints.Add(endpoint);
additionalUpdatedEndpoints.Add(endpoint);
}
}
}

UpdatedEndpoints = updatedEndpoints.Distinct().Select(e => e.ToTaskItem()).ToArray();
updatedEndpoints.UnionWith(additionalUpdatedEndpoints);

UpdatedEndpoints = StaticWebAssetEndpoint.ToTaskItems(updatedEndpoints);

return true;
}

private static HashSet<string> GetCompressedHeaders(StaticWebAssetEndpoint compressedEndpoint)
{
var result = new HashSet<string>(compressedEndpoint.ResponseHeaders.Length, StringComparer.Ordinal);
for (var i = 0; i < compressedEndpoint.ResponseHeaders.Length; i++)
{
var responseHeader = compressedEndpoint.ResponseHeaders[i];
result.Add(responseHeader.Name);
}

return result;
}

private static Dictionary<string, List<StaticWebAssetEndpoint>> GetEndpointsByRoute(
IDictionary<string, List<StaticWebAssetEndpoint>> endpointsByAsset)
{
var result = new Dictionary<string, List<StaticWebAssetEndpoint>>(endpointsByAsset.Count);

foreach (var endpointsList in endpointsByAsset.Values)
{
foreach (var endpoint in endpointsList)
{
if (!result.TryGetValue(endpoint.Route, out var routeEndpoints))
{
routeEndpoints = new List<StaticWebAssetEndpoint>(5);
result[endpoint.Route] = routeEndpoints;
}
routeEndpoints.Add(endpoint);
}
}

return result;
}

private static StaticWebAssetEndpointResponseHeader[] GetOrCreateCompressionHeaders(Dictionary<string, StaticWebAssetEndpointResponseHeader[]> compressionHeadersByEncoding, StaticWebAsset compressedAsset)
{
if (!compressionHeadersByEncoding.TryGetValue(compressedAsset.AssetTraitValue, out var compressionHeaders))
{
compressionHeaders = CreateCompressionHeaders(compressedAsset);
compressionHeadersByEncoding.Add(compressedAsset.AssetTraitValue, compressionHeaders);
}

return compressionHeaders;
}

private static StaticWebAssetEndpointResponseHeader[] CreateCompressionHeaders(StaticWebAsset compressedAsset) =>
[
new()
{
Name = "Content-Encoding",
Value = compressedAsset.AssetTraitValue
},
new()
{
Name = "Vary",
Value = "Content-Encoding"
}
];

private StaticWebAssetEndpoint CreateUpdatedEndpoint(
StaticWebAsset compressedAsset,
string quality,
StaticWebAssetEndpoint compressedEndpoint,
HashSet<string> compressedHeaders,
StaticWebAssetEndpoint relatedEndpointCandidate)
{
Log.LogMessage(MessageImportance.Low, "Processing related endpoint '{0}'", relatedEndpointCandidate.Route);
var encodingSelector = new StaticWebAssetEndpointSelector
{
Name = "Content-Encoding",
Value = compressedAsset.AssetTraitValue,
Quality = quality
};
Log.LogMessage(MessageImportance.Low, " Created Content-Encoding selector for compressed asset '{0}' with size '{1}' is '{2}'", encodingSelector.Value, encodingSelector.Quality, relatedEndpointCandidate.Route);
var endpointCopy = new StaticWebAssetEndpoint
{
AssetFile = compressedAsset.Identity,
Route = relatedEndpointCandidate.Route,
Selectors = [
..relatedEndpointCandidate.Selectors,
encodingSelector
],
EndpointProperties = relatedEndpointCandidate.EndpointProperties
};
var headers = new List<StaticWebAssetEndpointResponseHeader>(7);
Copy link
Member

@MackinnonBuck MackinnonBuck Apr 21, 2025

Choose a reason for hiding this comment

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

Why 7? How would we know when to update this in the future?

Copy link
Member Author

@javiercn javiercn Apr 22, 2025

Choose a reason for hiding this comment

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

It's the number of headers we add at most by default.

ApplyCompressedEndpointHeaders(headers, compressedEndpoint, relatedEndpointCandidate.Route);
ApplyRelatedEndpointCandidateHeaders(headers, relatedEndpointCandidate, compressedHeaders);
endpointCopy.ResponseHeaders = [.. headers];

// Update the endpoint
Log.LogMessage(MessageImportance.Low, " Updated related endpoint '{0}' with Content-Encoding selector '{1}={2}'", relatedEndpointCandidate.Route, encodingSelector.Value, encodingSelector.Quality);
return endpointCopy;
}

private static bool HasContentEncodingResponseHeader(StaticWebAssetEndpoint compressedEndpoint)
{
for (var i = 0; i < compressedEndpoint.ResponseHeaders.Length; i++)
{
var responseHeader = compressedEndpoint.ResponseHeaders[i];
if (string.Equals(responseHeader.Name, "Content-Encoding", StringComparison.Ordinal))
{
return true;
}
}

return false;
}

private static bool HasContentEncodingSelector(StaticWebAssetEndpoint compressedEndpoint)
{
for (var i = 0; i < compressedEndpoint.Selectors.Length; i++)
{
var selector = compressedEndpoint.Selectors[i];
if (string.Equals(selector.Name, "Content-Encoding", StringComparison.Ordinal))
{
return true;
}
}

return false;
}

private (List<StaticWebAssetEndpoint> compressedEndpoints, List<StaticWebAssetEndpoint> relatedAssetEndpoints) ResolveEndpoints(
IDictionary<string, StaticWebAsset> assetsById,
IDictionary<string, List<StaticWebAssetEndpoint>> endpointsByAsset,
StaticWebAsset compressedAsset)
{
if (!assetsById.TryGetValue(compressedAsset.RelatedAsset, out var relatedAsset))
{
Log.LogWarning("Related asset not found for compressed asset: {0}", compressedAsset.Identity);
throw new InvalidOperationException($"Related asset not found for compressed asset: {compressedAsset.Identity}");
}

if (!endpointsByAsset.TryGetValue(compressedAsset.Identity, out var compressedEndpoints))
{
Log.LogWarning("Endpoints not found for compressed asset: {0} {1}", compressedAsset.RelativePath, compressedAsset.Identity);
throw new InvalidOperationException($"Endpoints not found for compressed asset: {compressedAsset.Identity}");
}

if (!endpointsByAsset.TryGetValue(relatedAsset.Identity, out var relatedAssetEndpoints))
{
Log.LogWarning("Endpoints not found for related asset: {0}", relatedAsset.Identity);
throw new InvalidOperationException($"Endpoints not found for related asset: {relatedAsset.Identity}");
}

return (compressedEndpoints, relatedAssetEndpoints);
}

private static string ResolveQuality(StaticWebAsset compressedAsset) =>
Math.Round(1.0 / (compressedAsset.FileLength + 1), 12).ToString("F12", CultureInfo.InvariantCulture);

private static bool IsCompatible(StaticWebAssetEndpoint compressedEndpoint, StaticWebAssetEndpoint relatedEndpointCandidate)
{
var compressedFingerprint = compressedEndpoint.EndpointProperties.FirstOrDefault(ep => ep.Name == "fingerprint");
var relatedFingerprint = relatedEndpointCandidate.EndpointProperties.FirstOrDefault(ep => ep.Name == "fingerprint");
var compressedFingerprint = ResolveFingerprint(compressedEndpoint);
var relatedFingerprint = ResolveFingerprint(relatedEndpointCandidate);
return string.Equals(compressedFingerprint.Value, relatedFingerprint.Value, StringComparison.Ordinal);
}

private static StaticWebAssetEndpointProperty ResolveFingerprint(StaticWebAssetEndpoint compressedEndpoint)
{
foreach (var property in compressedEndpoint.EndpointProperties)
{
if (string.Equals(property.Name, "fingerprint", StringComparison.Ordinal))
{
return property;
}
}
return default;
}

private void ApplyCompressedEndpointHeaders(List<StaticWebAssetEndpointResponseHeader> headers, StaticWebAssetEndpoint compressedEndpoint, string relatedEndpointCandidateRoute)
{
foreach (var header in compressedEndpoint.ResponseHeaders)
Expand Down
Loading