Skip to content

Stop NGENing System.Net.Http and dependencies#27537

Merged
OmarTawfik merged 3 commits intodotnet:dev15.8-preview3from
OmarTawfik:dev/otawfik/stop-net-http-ngen
Jun 7, 2018
Merged

Stop NGENing System.Net.Http and dependencies#27537
OmarTawfik merged 3 commits intodotnet:dev15.8-preview3from
OmarTawfik:dev/otawfik/stop-net-http-ngen

Conversation

@OmarTawfik
Copy link
Copy Markdown
Contributor

@OmarTawfik OmarTawfik commented Jun 6, 2018

This is failing NGEN in official builds, as binding redirects to dependencies are not generated correctly.
We don't actually use/load that assembly in any way, so let's stop NGENing it.

cc @jaredpar @tannergooding @jasonmalinowski

@OmarTawfik OmarTawfik added this to the 15.8 milestone Jun 6, 2018
@OmarTawfik OmarTawfik requested a review from a team as a code owner June 6, 2018 21:15
@OmarTawfik OmarTawfik requested a review from jaredpar June 6, 2018 21:17
@tannergooding
Copy link
Copy Markdown
Member

tannergooding commented Jun 6, 2018

Could you retarget to dev15.8.x-preview3-vs-deps for the insertion?

@jasonmalinowski jasonmalinowski changed the base branch from master to dev15.8-preview3 June 6, 2018 21:21
jasonmalinowski
jasonmalinowski reviewed Jun 6, 2018
View reviewed changes
Comment thread src/Setup/DevDivVsix/CompilersPackage/Microsoft.CodeAnalysis.Compilers.swr Outdated
file source=$(OutputPath)\Vsix\CompilerExtension\System.AppContext.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.Console.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.Diagnostics.DiagnosticSource.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.Diagnostics.DiagnosticSource.dll
Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski Jun 6, 2018

Choose a reason for hiding this comment

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

We should really put comments here why these are different.

Copy link
Copy Markdown
Member

@jaredpar jaredpar Jun 6, 2018

Choose a reason for hiding this comment

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

Is there a way to explicitly opt out of NGEN vs. implicitly doing so?

@jaredpar
Copy link
Copy Markdown
Member

jaredpar commented Jun 6, 2018

Related dotnet/sdk#2227

@OmarTawfik
Copy link
Copy Markdown
Contributor Author

OmarTawfik commented Jun 6, 2018

There is a build boss error:

14:27:29 Verifying Microsoft.CodeAnalysis.Compilers.swr
14:27:29 	Dll System.Diagnostics.DiagnosticSource.dll is missing
14:27:29 	Dll System.Net.Http.dll is missing

Investigating why.

@OmarTawfik OmarTawfik requested a review from a team as a code owner June 6, 2018 21:48
@OmarTawfik OmarTawfik merged commit 44f2bca into dotnet:dev15.8-preview3 Jun 7, 2018
@OmarTawfik OmarTawfik deleted the dev/otawfik/stop-net-http-ngen branch June 7, 2018 01:37
@tmat
Copy link
Copy Markdown
Member

tmat commented Jun 7, 2018

Why do we still include them if we know we're not gonna use them?

@jaredpar
Copy link
Copy Markdown
Member

jaredpar commented Jun 7, 2018

@tmat see dotnet/sdk#2227

This is what the SDK believes we are using. Previously we tried to be smart and exclude items we thought we weren't using. That lead to a series of different issues. There just doesn't appear to be a way to win here.

@OmarTawfik OmarTawfik mentioned this pull request Jun 7, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaredpar jaredpar jaredpar approved these changes

@jasonmalinowski jasonmalinowski jasonmalinowski approved these changes

@tannergooding tannergooding tannergooding approved these changes

Assignees

No one assigned

Labels

Area-Compilers Bug

Projects

None yet

Milestone

15.8

Development

Successfully merging this pull request may close these issues.

5 participants

@OmarTawfik @tannergooding @jaredpar @tmat @jasonmalinowski