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.

Sign up for GitHub

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

Jump to bottom

[Feature] Replace glob with fast-glob for .*proj file search in project-graph calculation #410

Closed
johannesf95 opened this issue Apr 13, 2022 · 5 comments · Fixed by #414
Closed

[Feature] Replace glob with fast-glob for .*proj file search in project-graph calculation #410

johannesf95 opened this issue Apr 13, 2022 · 5 comments · Fixed by #414
Assignees
@johannesf95
Labels
enhancement New feature or request outdated scope: core

Comments

@johannesf95
Copy link
Contributor

johannesf95 commented Apr 13, 2022 •
edited
Loading

Is your feature request related to a problem? Please describe.
When trying to introduce nx with nx-dotnet to a large existing solution with about 70 projects, I figured out that the project graph calculation took several minutes. By debugging the nx-dotnet plugin I found out that the glob search for .*proj files took about 3 minutes, due to the fact that I did not use the nx dist dir as output directory but the normal bin directory within the project directory to avoid to much changes to the repository structure and build architecture for now.

Describe the solution you'd like

  1. By replacing glob with fast-glob locally, I was able to reduce the execution time of the project-graph calculation from ~3 Min. to 30 Sec. So I would like to suggest using fast-glob which is used by nx as well.
  2. Optionally: By passing the ignore option to glob / fast-glob like: ['**/bin/**', '**/obj/**'], it is possible to speed up the calculation even more: fast-glob took 15 Sec. whereas glob took ~35 Sec.

Describe alternatives you've considered
Changing the output directories to the normal nx dist directory could be an option to speed up the execution, but that would possibly interfere with our current build architecture and would cause more concerns from some developers against introducing nx workspace.
@johannesf95 johannesf95 added enhancement New feature or request needs-triage This issue has yet to be looked over by a core team member labels Apr 13, 2022
@johannesf95 johannesf95 changed the title [Feature] Replace glob with fast-glob for *. [Feature] Replace glob with fast-glob for .*proj file search in project-graph calculation Apr 13, 2022
@AgentEnder AgentEnder added scope: core and removed needs-triage This issue has yet to be looked over by a core team member labels Apr 13, 2022
@AgentEnder
Copy link
Member

Sounds good to me. Do you care to throw a PR together for this? If you've done it locally, it shouldn't be too much trouble. Use the same version of fast-glob we use in Nx; there are some regression bugs in the latest version.

@johannesf95
Copy link
Contributor Author

Sure, will try to open a PR tomorrow! 👍

@AgentEnder
Copy link
Member

Regardless of the swap to fast glob, we should do option 2 eventually as well. Nx does this during project inference, but we should adhere to it as well.

johannesf95 added a commit to johannesf95/nx-dotnet that referenced this issue Apr 14, 2022
@johannesf95
feat(utils): replace glob with fast-glob (nx-dotnet#410)
478c19c
@johannesf95 johannesf95 mentioned this issue Apr 14, 2022
Merged
AgentEnder pushed a commit that referenced this issue Apr 14, 2022
@johannesf95
fix(core): replace glob with fast-glob to speed up dep-graph calculat…
5db4ca9 
…ion (#414)

Closes #410
github-actions bot pushed a commit that referenced this issue Apr 21, 2022
@semantic-release-bot
release: 1.9.11 [skip ci]
6e084ea 
## [1.9.11](v1.9.10...v1.9.11) (2022-04-21)

### Bug Fixes

* **core:** replace glob with fast-glob to speed up dep-graph calculation ([#414](#414)) ([5db4ca9](5db4ca9)), closes [#410](#410)
* **dotnet:** expand env vars in cli parameters ([#422](#422)) ([c2db0cd](c2db0cd))

Apr 21, 2022, 2:46 PM
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 1.9.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@johannesf95 johannesf95

Labels
enhancement New feature or request outdated scope: core
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(utils): replace glob with fast-glob johannesf95/nx-dotnet
2 participants
@AgentEnder @johannesf95