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

Consider to have an option to disable loading additional TFM in a super large solution #3323

Open
lifengl opened this issue Feb 28, 2018 · 14 comments
Assignees
Labels
Feature-Multi-Targeting Targeting and building for multiple frameworks. Triage-Approved Reviewed and prioritized VS1ES Issues that affect internal teams at Microsoft using VS in our engineering system

Comments

@lifengl
Copy link
Contributor

lifengl commented Feb 28, 2018

When many projects in the solution have multiple TFMs, the current way to load all of them into memory and runs design time builds and builds language model is not scale well. Instead of running out of memory or make the IDE very slow, may suppressing some design time support, but fully support TFMs during build can be an option.

@davkean davkean self-assigned this Feb 28, 2018
@davkean davkean added this to the 15.7 milestone Feb 28, 2018
@davkean
Copy link
Member

davkean commented Feb 28, 2018

Yep, this requires a little design to figure out.

@davkean
Copy link
Member

davkean commented Mar 14, 2018

I'll make this the temporary opt-out. We need a long term design for the real fix which might be to dynamically load/unload configs based on usage.

@davkean davkean added the Blocking-Release We cannot ship the current milestone without this feature. label Mar 14, 2018
@steffenloesch
Copy link

This would help us a lot I think. It would be great to have a way to select what 'main TFM' should be, and to adjust that based on editing needs. It would be acceptable for us to require a restart of VS for the changes to become effective.

@davkean
Copy link
Member

davkean commented Mar 19, 2018

@steffenloesch The main TFM will be the first, that's how we use it for all others that don't understand "multi-targeting".

@steffenloesch
Copy link

So when a developer wants to switch the main TFM to get a better IDE experience for that TFM, she'd have to reorder the TFM list, right? We can probably work with that.

@davkean
Copy link
Member

davkean commented Mar 20, 2018

I think I have a design for this and turns out to be pretty easy:

  • In TargetFrameworkProjectConfigurationDimensionProvider, when this option is set; we'll return just the first TFM from the list <TargetFrameworks/>. The rest of the IDE will still think it's in multi-targeting mode, and hence things like solution build will continue to work (where it clears "TFM" to build from the outer), but all the other features will just think they are playing with a single TFM.

@Pilchie Pilchie added the VS1ES Issues that affect internal teams at Microsoft using VS in our engineering system label Apr 25, 2018
@davkean
Copy link
Member

davkean commented Apr 26, 2018

This design is harder than we think; we still an evaluation & design-time build to make NuGet restore happy, so we can't just turn off the configuration.

@davkean
Copy link
Member

davkean commented Apr 26, 2018

@steffenloesch: Note for 15.7 Preview 5 - we've made a change that prevent us from loading an additional config for projects if <TargetFrameworks></TargetFrameworks> is defined within the project itself: #3482.

@Pilchie Pilchie modified the milestones: 15.7, 15.8 Apr 26, 2018
@Pilchie
Copy link
Member

Pilchie commented Apr 26, 2018

Unfortunately, moving the remainder to 15.8.

@steffenloesch
Copy link

Thanks for the updates! @davkean can you be more specific about what we need to do to avoid unnecessary loading? Does this change only work if the TargetFrameworks are explicitly defined in the csproj itself, or do you also support defining it in Directory.Build.props, say? Your last post can be read as "TargetFrameworks needs to be set to empty", which doesn't make sense to me. Do Platform and Configuration also have to be explicit in the csproj itself for getting efficient loading?

@davkean
Copy link
Member

davkean commented Apr 27, 2018

The optimization applies if TargetFrameworks appears within the project itself, it shouldn't be empty. We've yet to come up with a design for looking in Directory.Build.props.

@steffenloesch
Copy link

Follow-up question for @davkean: Is it sufficient for this optimization that first target framework is defined explicitly in the csproj, but others are not? In other words: if my project file contains
<TargetFrameworks>net472;$(MyOtherTargetFramework)</TargetFrameworks>, would it benefit from the perf improvement that came in with VS 15.7 Preview 5?

@davkean
Copy link
Member

davkean commented May 3, 2018

Yes, we will handle that correctly.

@steffenloesch
Copy link

@davkean PR #3482 mentions "Configuration and Platform are only read if the solution didn't provide a default". What entries need to be present in the sln file to provide a default? Is it entries in GlobalSection(ProjectConfigurationPlatforms) like this?
GlobalSection(ProjectConfigurationPlatforms) = postSolution {79243C18-8A45-41DC-A39E-03FBD110BD13}.Debug|x64.ActiveCfg = Debug|x64

@davkean davkean modified the milestones: 15.8, 16.0 May 24, 2018
@davkean davkean removed the Blocking-Release We cannot ship the current milestone without this feature. label May 24, 2018
@davkean davkean added the Feature-Multi-Targeting Targeting and building for multiple frameworks. label Dec 12, 2018
@jjmew jjmew added the Triage-Approved Reviewed and prioritized label Dec 17, 2018
@jjmew jjmew modified the milestones: 16.0, 16.X Feb 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Multi-Targeting Targeting and building for multiple frameworks. Triage-Approved Reviewed and prioritized VS1ES Issues that affect internal teams at Microsoft using VS in our engineering system
Projects
None yet
Development

No branches or pull requests

6 participants