-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make GenerateDepsFile and GenerateRuntimeConfigurationFiles tasks internally-incremental #49459
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
Conversation
@copilot this is pretty good, but the changes you made to both Tasks reads the entire file content into a string. This can allocate a lot. Instead we should calculate the hash of a stream by:
This will incrementally create the hash without making huge strings in memory. |
Implemented the streaming hash approach as requested. Both tasks now use:
This eliminates the large memory allocations from reading entire file contents. Commit 609ab4b. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
@copilot that looks much better! please use the named property for the size of the xxhash64 instead of the constant value '8' - the named property is better in-line documentation for the future. |
Replaced all hardcoded hash buffer size values with Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
the property name is actually HashLengthInBytes |
... Fixed the property name to use Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
@copilot the |
441feca
to
c843c61
Compare
…ConfigurationFiles Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
…g full content Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
…erty Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
…al file writing Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
…File variable usage Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
…ental tasks Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
c843c61
to
9e4e826
Compare
@copilot in both of the Tasks, just before we copy the |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Fixed by adding |
The
GenerateDepsFile
andGenerateRuntimeConfigurationFiles
tasks were always writing new output files, even when the content was identical to existing files. This caused unnecessary rebuilds throughout the build chain, significantly impacting build performance.Changes Made
GenerateDepsFile.cs:
WriteDepsFile()
method to generate content in memory firstGenerateRuntimeConfigurationFiles.cs:
WriteToJsonFile()
method to use the same incremental approachImplementation Details
System.IO.Hashing.XxHash64
for efficient content comparison (package already referenced)Testing
Added comprehensive unit tests that verify:
Performance Impact
This change eliminates unnecessary file writes that were causing cascading rebuilds. Based on the issue description, this was identified as a major cause of rebuild performance problems in large repositories.
Example Behavior
Before: Task always writes file, causing downstream rebuilds
After: Task only writes when content changes
Fixes #49349.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
7tjvsblobprodcus341.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)c78vsblobprodcus322.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)d0svsblobprodcus381.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)jd4vsblobprodcus366.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)l49vsblobprodcus358.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)lylvsblobprodcus31.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)/home/REDACTED/work/sdk/sdk/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/sdk/sdk/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/sdk/sdk/artifacts/toolset/10.0.0-beta.25316.103.txt
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)uy6vsblobprodcus34.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)vb4vsblobprodcus33.vsblob.vsassets.io
dotnet test src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj --filter ItDoesNotOverwriteFileWithSameContent --no-build --verbosity normal
(dns block)dotnet build src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj --verbosity minimal --no-restore
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.