Skip to content

Fix NGEN priority of csc.exe, vbc.exe, csi.exe#40016

Merged
tmat merged 1 commit intodotnet:masterfrom
tmat:FixNgenPri
Nov 26, 2019
Merged

Fix NGEN priority of csc.exe, vbc.exe, csi.exe#40016
tmat merged 1 commit intodotnet:masterfrom
tmat:FixNgenPri

Conversation

@tmat
Copy link
Copy Markdown
Member

@tmat tmat commented Nov 26, 2019

@tmat tmat requested a review from a team as a code owner November 26, 2019 01:21
@tmat
Copy link
Copy Markdown
Member Author

tmat commented Nov 26, 2019

@jaredpar @chsienki

@tmat tmat merged commit d224495 into dotnet:master Nov 26, 2019
@tmat tmat deleted the FixNgenPri branch November 26, 2019 03:12

<!-- Note: do not use Update attribute (see https://github.com/microsoft/msbuild/issues/1124) -->
<DesktopCompilerArtifact NgenPriority="1" Condition="'%(Identity)' == '$(ArtifactsBinDir)csi\$(Configuration)\net472\System.Collections.Immutable.dll'" />
<DesktopCompilerArtifact NgenPriority="1" Condition="'%(Identity)' == '$(ArtifactsBinDir)csi\$(Configuration)\net472\System.Reflection.Metadata.dll'" />
Copy link
Copy Markdown
Member

@jaredpar jaredpar Nov 26, 2019

Choose a reason for hiding this comment

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

@tmat can you elaborate more on why this fixed the problem? I looked at the referenced issue in MSBuild and it's still not obvious to me what is going on here. My reading of this code is still that the NgenPriority attribute should only be updated for the cases where the specified Update string was matched but this apparently is not the case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The bug in msbuild is that the Update attribute is ignored. So the previous code was equivalent to <DesktopCompilerArtifact NgenPriority="1"/>, which to msbuild means set NgenPriority to 1 on all existing items DesktopCompilerArtifact. Adding the condition limits this to only those items whose identity is as specified, which is what we intended to do using the Update attribute.

333fred added a commit to 333fred/roslyn that referenced this pull request Dec 2, 2019
…ointers

* dotnet/master: (197 commits)
  Update dependencies from https://github.com/dotnet/arcade build 20191201.1 (dotnet#40084)
  Update list of C# Next features and reviewers (dotnet#39987)
  Update dependencies from https://github.com/dotnet/arcade build 20191130.1 (dotnet#40075)
  [master] Update dependencies from dotnet/arcade (dotnet#40065)
  Update dependencies from https://github.com/dotnet/arcade build 20191127.4 (dotnet#40057)
  Added more tests & fixed minor bug
  Deterministic ordering + added tests back
  Update dependencies from https://github.com/dotnet/arcade build 20191126.2 (dotnet#40036)
  removed a test due to random order generation
  Always restore when running a bootstrap build (dotnet#40025)
  fixed integration test capitalization for extract method and extract interface
  Fix tests pt2
  DiagnosticIncrementalAnalyzer refactoring (dotnet#39956)
  Persistence is always available in OOP
  Update src/Workspaces/Core/Portable/SolutionSize/SolutionSizeTracker.cs
  Fixed tests
  Move reporting of iterator diagnostics to avoid race condition (dotnet#39990)
  Update dependencies from https://github.com/dotnet/arcade build 20191125.7 (dotnet#40020)
  Fix NGEN priority of csc.exe, vbc.exe, csi.exe (dotnet#40016)
  Accidentally deleted something
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants