Skip to content

Upgrade System.Collections.Immutable and System.Reflection.Metadata#22845

Closed
jasonmalinowski wants to merge 6 commits intomasterfrom
dev/jasonmal/update-binaries-for-new-optprof-data
Closed

Upgrade System.Collections.Immutable and System.Reflection.Metadata#22845
jasonmalinowski wants to merge 6 commits intomasterfrom
dev/jasonmal/update-binaries-for-new-optprof-data

Conversation

@jasonmalinowski
Copy link
Member

These new packages contain the exact same bits (and same assembly versions) as the previous packages, but with updated partial ngen data.


Customer scenario: load a WinForms designer, and lots of JITting happens.
Bugs this fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/433634
Workarounds, if any: none
Risk: very low, packages contain new partial ngen data but should otherwise contain the same code.
Performance impact: better; this improves some load times by around 100ms in Visual Studio.
Is this a regression from a previous update? was regressed around the 15.0 time frame, and caught late.
Root cause analysis: dependencies moved and changed. The exact root cause was never understood.
How was the bug found? RPS.

@jasonmalinowski jasonmalinowski force-pushed the dev/jasonmal/update-binaries-for-new-optprof-data branch 3 times, most recently from 63f7011 to 228e72d Compare October 27, 2017 17:21
@jasonmalinowski jasonmalinowski self-assigned this Oct 27, 2017
These new packages contain the exact same bits (and same assembly
versions) as the previous packages, but with updated partial ngen data.
This contained a list of packages that we depended on and needed
facades deployed into Visual Studio. We already have
VisualStudioSetup.Dependencies that already (should have...) contained
all the facades and more. We can just consume that project instead
of having yet another duplication of facade lists.
…t include

This ensures that if the compiler adds a new facade but we don't update
VisualStudioSetup.Dependencies (either intentionally or by accident)
we still will include the right facades.
By doing this, the VisualStudioSetup.Dependencies.csproj will now
figure out the highest version of our own dependencies and the debugger
dependencies, and create a single package that we can deploy that
contains all of them.
@jasonmalinowski jasonmalinowski force-pushed the dev/jasonmal/update-binaries-for-new-optprof-data branch from 228e72d to 6fe3cbd Compare October 31, 2017 17:06
file source="$(NuGetPackageRoot)\System.IO.FileSystem.Primitives\4.3.0\lib\net46\System.IO.FileSystem.Primitives.dll" vs.file.ngen=yes
file source="$(NuGetPackageRoot)\System.IO.Pipes\4.3.0\runtimes\win\lib\net46\System.IO.Pipes.dll" vs.file.ngen=yes
file source="$(NuGetPackageRoot)\System.Net.Security\4.3.0\runtimes\win\lib\net46\System.Net.Security.dll" vs.file.ngen=yes
file source="$(NuGetPackageRoot)\System.Net.Security\4.3.0\lib\net46\System.Net.Security.dll" vs.file.ngen=yes
Copy link
Member Author

Choose a reason for hiding this comment

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

@ericstj We're looking at the output of a project.assets.json and seeing that for the runtime facades for targeting 4.6 we're seeing the package contains both stuff in lib and stuff in runtimes. We're not sure why the tooling is changing this, but can't explain why the package has both files either. Are they the same? Interchangeable?

@jasonmalinowski
Copy link
Member Author

FYI to @jaredpar just because I know you're trying to make heads or tails of some of the insertion stuff right now.

@jasonmalinowski
Copy link
Member Author

We are no longer upgrading to these versions of the packages, but the infrastructure improvements and fixing our dependency stuff has been put into #25609.

@jasonmalinowski jasonmalinowski deleted the dev/jasonmal/update-binaries-for-new-optprof-data branch March 19, 2018 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants