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

Slow performance using dotnet restore & build with deep folder structure #7725

Closed
vlesierse opened this issue Feb 10, 2017 · 20 comments
Closed
Milestone

Comments

@vlesierse
Copy link

Steps to reproduce

Create a new .NET Core project

mkdir MyProject
cd MyProject
dotnet new mvc

Create an embedded client app using create-react-app

mkdir client
cd client
create-react-app .

Restore the .NET Core application

cd ..
dotnet restore MyProject.csproj

Expected behavior

The expectation is that the restore should be very fast or at least as fast with the project.json version of the sdk.

Actual behavior

Restoring the application's packages is terribly slow, probably due to the deep directory structure which node creates.

Environment data

dotnet --info output:
.NET Command Line Tools (1.0.0-rc4-004771)

Product Information:
Version: 1.0.0-rc4-004771
Commit SHA-1 hash: 4228198

Runtime Environment:
OS Name: Mac OS X
OS Version: 10.12
OS Platform: Darwin
RID: osx.10.12-x64
Base Path: /usr/local/share/dotnet/sdk/1.0.0-rc4-004771

@blackdwarf
Copy link

@vlesierse what does create-react-app . do? Drop a package.json and then do a npm install or something similar? I assume that the node_modules is in the client directory?

@vlesierse
Copy link
Author

vlesierse commented Feb 10, 2017

Exactly. It's likedotnet new for the package.json, node_modules and a template react app...

Think that npm init && npm install ..... has the same effect.

Edit: It does. If you add more packages to npm the performance will degrade. Previous versions of the SDK with project.json didn't had this problem...

@blackdwarf
Copy link

@rohit21agrawal @emgarten FYI.

@emgarten
Copy link
Member

@vlesierse is the react app adding anything that would be handled by MSBuild? Does removing the deep sub folder improve performance?

@vlesierse
Copy link
Author

It's a set of around 1000 npm dependencies. As of npm 3 the node_modules is flat but still contains 1000 folders. When I remove the node_modules folder the dotnet commands are fast again. I've scanned to MSBuild specific files in the sub folder but there are none. js files and some compiled libraries.

The screenshot shows the CPU is piking during the restore and it takes about a minute before it even starts downloading. My guess is it searches for files in the directory structure which isn't that efficient...

screen shot 2017-02-10 at 20 54 36

@emgarten
Copy link
Member

@vlesierse thanks for the info, do you see the same slow behavior when running other targets such as /t:Clean?

@vlesierse
Copy link
Author

vlesierse commented Feb 10, 2017

Yes, clean has the same behaviour... dotnet clean

@dasMulli
Copy link
Contributor

dasMulli commented Feb 10, 2017

Looks like msbuild is taking long to expand the globs brought in via the web sdk. The targets themselves don't take very long to run (note that I never ran restore or build, just clean):

MacBook-Pro:tmp martin$ time dotnet msbuild /t:Clean /nologo /clp:PerformanceSummary

Project Performance Summary:
       55 ms  /Users/martin/tmp/tmp.csproj               1 calls
                 55 ms  Clean                                      1 calls

Target Performance Summary:
        0 ms  Clean                                      1 calls
        0 ms  AfterClean                                 1 calls
        0 ms  PrepareProjectReferences                   1 calls
        0 ms  BeforeClean                                1 calls
        0 ms  CleanReferencedProjects                    1 calls
        0 ms  _SplitProjectReferencesByFileExistence     1 calls
        0 ms  _CheckRuntimeIdentifier                    1 calls
        1 ms  CleanPublishFolder                         1 calls
        3 ms  _GetProjectReferenceTargetFrameworkProperties   1 calls
        5 ms  CoreClean                                  1 calls
        6 ms  CheckForDuplicateItems                     1 calls
        8 ms  _CheckForUnsupportedTargetFramework        1 calls
        8 ms  _CheckForInvalidConfigurationAndPlatform   1 calls
       15 ms  CheckForImplicitPackageReferenceOverrides   1 calls

Task Performance Summary:
        0 ms  RemoveDuplicates                           1 calls
        0 ms  WriteLinesToFile                           1 calls
        1 ms  MakeDir                                    1 calls
        1 ms  ReadLinesFromFile                          1 calls
        1 ms  Delete                                     2 calls
        1 ms  FindUnderPath                              2 calls
        1 ms  MSBuild                                    1 calls
        2 ms  Message                                    2 calls
        6 ms  CheckForDuplicateItems                     3 calls
        7 ms  CheckForImplicitPackageReferenceOverrides   1 calls

real	0m9.992s
user	0m6.952s
sys	0m3.141s

If I add <EnableDefaultItems>False</EnableDefaultItems> to the project, the same invocation only takes a second (a tenth of the time).

@vlesierse
Copy link
Author

@dasMulli I can confirm that <EnableDefaultItems>False</EnableDefaultItems> makes the execution fast again. Not only dotnet clean, but also dotnet restore and dotnet build. However it doesn't the build fails due to the missing *.cs files.

@TheRealPiotrP
Copy link
Contributor

@rainersigwald @srivatsn, looks like this should go to msbuild or SDK. Which repo is the winner?

@dasMulli
Copy link
Contributor

Assuming you only want to publish the built output and not all development folders, you can exclude folders from being published - including the node_modules folder.
If I add: <DefaultItemExcludes>$(DefaultItemExcludes);client\node_modules\**</DefaultItemExcludes> as property, dotnet publish only takes 3.2 seconds instead of 15.

@vlesierse
Copy link
Author

Works great as work around...

@rainersigwald
Copy link
Member

I don't think there's a great general-purpose solution for this. Adding to the default excludes or opting into explicit includes of narrower patterns (which I always recommend anyway, but isn't the default) allows you to specialize to suit your particular case.

One possibility would be for the Web Sdk to add **\node_modules\** to their version of the default exclude glob, since that shouldn't be a common pattern for non-Web projects. But what an ugly pattern to have to consider all the time.

(nice debugging @dasMulli!)

@vlesierse
Copy link
Author

vlesierse commented Feb 14, 2017

For web applications it's a save assumption to exclude the node_modules by default. If needed you can include specific targets inside the folders, but I think that is very unlikely.

@TheRealPiotrP
Copy link
Contributor

@mlorbetske

@davidfowl
Copy link
Member

I'm surprised we don't already do this.

@rainersigwald
Copy link
Member

{csproj folder}/node_modules/** is excluded by default. This one's in a subfolder so that doesn't apply.

I think it comes down to weighing

  • The fact that it'll almost always be correct to exclude **\node_modules\**
  • The relative frequency of having one in a subfolder
  • The cost of carrying around that recursive exclusion everywhere (not super duper high I imagine, but haven't measured)

@rhyek
Copy link

rhyek commented May 1, 2017

I have a Vue app at project_root\Clients\Web. I imagine it's a common thing to have something like this.

Adding <DefaultItemExcludes>$(DefaultItemExcludes);Clients\Web\node_modules\**</DefaultItemExcludes> solved the issue for me.

@dasMulli
Copy link
Contributor

dasMulli commented May 1, 2017

Looks like this has been fixed in the web sdk for the upcoming release https://github.com/aspnet/websdk/blob/dev/src/ProjectSystem/Microsoft.NET.Sdk.Web.ProjectSystem.Targets/netstandard1.0/Microsoft.NET.Sdk.Web.ProjectSystem.props#L15

@livarcocc
Copy link
Contributor

Closing this for now, since it has fixed. If someone still runs into any issues, please, re-activate.

@msftgits msftgits transferred this issue from dotnet/cli Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants