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

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

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Conversation

tmat
Copy link
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
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:/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
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
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:/dotnet/arcade build 20191201.1 (dotnet#40084)
  Update list of C# Next features and reviewers (dotnet#39987)
  Update dependencies from https:/dotnet/arcade build 20191130.1 (dotnet#40075)
  [master] Update dependencies from dotnet/arcade (dotnet#40065)
  Update dependencies from https:/dotnet/arcade build 20191127.4 (dotnet#40057)
  Added more tests & fixed minor bug
  Deterministic ordering + added tests back
  Update dependencies from https:/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:/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