Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Reduce extension footprint when not in repository context #1802

Merged
merged 10 commits into from
Aug 7, 2018

Conversation

jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Jul 25, 2018

The problem

@PooyaZv (Writing from VS platform team) wrote:

GitHub extension reads in 2.5 MB off of disk when I create a basic C# app (with just an empty Main method) that doesn't use GitHub. This is with a full install of Visual Studio 15.8 Preview 3.0 (all workloads/packages installed). Disk access especially during solution load can be costly. Therefore, it should be avoided if the project doesn't use a feature.

We appear to be loading 22 assemblies, even when a project isn't using GitHub (see #1799 for a breakdown).

What this PR does

  1. Only create text view margins when in the context of a Git repository
    Change InlineCommentMarginProvider and PullRequestFileMarginProvider to lazy load their MEF dependencies and only return a margin when in the context of a Git repository.

  2. Remove dependency on LogManager from GitContextPackage
    GitContextPackage is our custom UI context and is loaded whenever we're in the context of a Git repository. This a low level package and doesn't really need logging (which requires a few assemblies).

  3. Change GitContextPackage and VSGitExt to use UICONTEXT.RepositoryOpen instead of GitSccProvider
    It appears there is a UIContext for when a Git repository is open (UICONTEXT.RepositoryOpen). Previously we were using GitSccProvider, which loads whenever the Git SCC provider is available (which is most of the time).

  4. Stop using SharedDictionary.xaml for GitHub logo
    The GitHub logo used on View > Other Windows > GitHub and StartPage > GitHub was causing a cascade of assembly loads when the StartPage or GitHub command was visible. I've replaced the SharedDictionary.xaml reference with a plain XAML placeholder. This causes only the containing assembly to load.

Questions

Our custom UIContext is now the same as UICONTEXT.RepositoryOpen, but with the added restriction that the host process is Visual Studio (not Blend). I don't know if there is a built in UIContext that would allow us to do the same thing? @PooyaZv do you know?

To do

  • Replace placeholder XAML with proper GitHub logo

How to test

  1. Open Visual Studio with StartPage visible and the GitHub pane hidden
  2. Open the modules window (Ctrl D + M) and search for github
  • Expect only GitHub.VisualStudio.dll to have loaded (it contains the GitHub icon)
  1. Open a project that isn't under Git version control and open a source file
  • Expect GitHub.InlineReviews.dll, GitHub.Exports.dll and GitHub.Exports.Reactive.dll to have loaded (these contain our MEF margin providers and MEF interfaces)
  • No other assemblies should have loaded under the extension directory

image

Fixes #1799

Use all `Lazy<>` imports for ImportingConstructor.
Since IPullRequestSessionManager is a MEF component we can import
Lazy<IPullRequestSessionManager> directly rather than going through
IGitHubServiceProvider.GetService<IPullRequestSessionManager>().
No need to log when not in a Visual Studio process.
We can be more precise and only initialize VSGitExt when in the
RepositoryOpen UIContext rather than the GitSccProvider UIContext.
Only load the GitContextPackage  when there is an open repository.
Loading `mark_github` via `GitHub.UI;component/SharedDictionary.xaml`
is taking a noticeable amount of time and causing multiple assemblies
to load. Changed to placeholder shape in the hope that we can replace
this with an inline XAML GitHub logo.
@jcansdale
Copy link
Collaborator Author

We appear to be loading a bunch of assemblies when simply viewing a button with the GitHub icon. This impacts both the View > Other Windows > GitHub and worse the StartPage > GitHub icon.

image

image

2018-07-25_13-42-35

@PooyaZv
Copy link

PooyaZv commented Jul 25, 2018

@jcansdale Thank you :) Can you please add @jialongcheng as a reviewer as well? He has more context on UI contexts than I.

@jcansdale
Copy link
Collaborator Author

jcansdale commented Jul 25, 2018

@PooyaZv, I tried adding @jialongcheng as a reviewer, but for some reason wasn't able to. 😕

@jialongcheng, do you know if there are any UIContexts I could use to differentiate between the Visual Studio process and Blend? Also, are there UIContexts for different versions of Visual Studio or would I need to use DTE.Version == "15.0"?

Bring the GitHub mark back into the menu (and start page)
@jcansdale jcansdale changed the title [wip] Reduce extension footprint when not in repository context Reduce extension footprint when not in repository context Jul 25, 2018
@jialongcheng
Copy link

@jcansdale we do not have a public UI context for checking if we are in Blend. What is your use case?
We do not have UI context for which version of VS is used.

@jcansdale
Copy link
Collaborator Author

@jialongcheng,

we do not have a public UI context for checking if we are in Blend. What is your use case?

The challenge is actually preventing the extension from running inside Blend. It appears that VS services are disabled inside Blend, but packages and MEF components are still loaded. We need to be very careful not to break when all of our services return null. 😢

Is there any way to prevent an extension's packages and MEF components from being loaded inside Blend? That would be ideal.

@jialongcheng
Copy link

@jcansdale Unfortunately we do not have a way at this point to prevent an extension's packages and MEF components from being loaded inside Blend.

@meaghanlewis meaghanlewis added this to the 2.5.5 milestone Jul 31, 2018
@grokys
Copy link
Contributor

grokys commented Aug 6, 2018

Trying this out, I get the following on running the experimental instance:

image

This is more than you indicated should be loaded. Is this correct?

This PR definitely does load less assemblies than before when a non-git repo project is opened though! Nice work!

@jcansdale
Copy link
Collaborator Author

jcansdale commented Aug 7, 2018

Trying this out, I get the following on running the experimental instance

I've just done some tests and was initially seeing the same as you. When you launch Visual Studio, make sure the Start Page, Team Explorer and GitHub tool windows are all closed. If all goes to plan, you should see this:

image

Show the Start Page:

image

Open a solution+source file that doesn't use Git:

image
(The InlineReviews project installs its MEF based editor margins)

Open a solution that in on GitHub:

image

Open the Team Explorer tool window. No change.

Open the GitHub tool window:

image

@jcansdale
Copy link
Collaborator Author

If you open a repository that is on Git but not GitHub, you'll see this:

image

This isn't great, but maybe something for another PR? It would need to probe for GitHub Enterprise to work properly.

Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

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

Yep, my mistake was having TE/StartPage open. This works great!

@jcansdale jcansdale merged commit 01afc6f into master Aug 7, 2018
@jcansdale jcansdale deleted the fixes/1799-reduce-footprint-non-GitHub-projects branch August 7, 2018 11:13
@meaghanlewis meaghanlewis removed this from the 2.5.5 milestone Aug 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants