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

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Apr 1, 2025

  • Removes LINQ usages.
  • Avoids excessive array amortization.
  • Reduces allocations

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member labels Apr 1, 2025
Copy link
Contributor

Thanks for your PR, @@javiercn.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

@javiercn javiercn changed the base branch from javiercn/reuse-original-task-item-for-unmodified-assets-and-endpoints to main April 1, 2025 19:43
@javiercn javiercn force-pushed the javiercn/more-cleanups branch from 4c1beb7 to 9e1de78 Compare April 4, 2025 09:39
@@ -115,16 +115,22 @@ public override bool Execute()
// Process the final set of candidate assets, deduplicating assets to be compressed in the same format multiple times and
// generating new a static web asset definition for each compressed item.
var formats = SplitPattern(Formats);
var assetsToCompress = new List<ITaskItem>();
var assetsToCompress = new ITaskItem[matchingCandidateAssets.Count * formats.Length];
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of creating a list and having to turn it into an array, we create an array of the max size we expect to hold and return that directly.

This takes advantage of the fact that MSBuild will ignore any item that is null when reading the outputs from a task, which save us from having to reallocate the list and convert it back into an array of contiguous elements

@javiercn javiercn changed the title Javiercn/more cleanups [StaticWebAssets] Improve allocations on ApplyCompressionNegotiation.cs and ResolveCompressedAssets.cs Apr 4, 2025
@javiercn javiercn changed the title [StaticWebAssets] Improve allocations on ApplyCompressionNegotiation.cs and ResolveCompressedAssets.cs [StaticWebAssets] Cleanup ApplyCompressionNegotiation.cs and ResolveCompressedAssets.cs Apr 4, 2025
@javiercn javiercn marked this pull request as ready for review April 4, 2025 17:00
@javiercn javiercn requested a review from a team as a code owner April 4, 2025 17:00
@javiercn javiercn requested a review from MackinnonBuck April 4, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant