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

v5 becomes significantly slower if a lot of NPM packages are installed #129

Closed
MuTsunTsai opened this issue Apr 1, 2024 · 5 comments · Fixed by #131
Closed

v5 becomes significantly slower if a lot of NPM packages are installed #129

MuTsunTsai opened this issue Apr 1, 2024 · 5 comments · Fixed by #131

Comments

@MuTsunTsai
Copy link

MuTsunTsai commented Apr 1, 2024

What were you expecting to happen?

The performance of Gulp v5 shouldn't be effected by installing additional NPM packages.

What actually happened?

Gulp v5 becomes noticable slower merely by having more NPM packages installed, especially if one uses PNPM as package manager.

Please give us a sample of your gulpfile

https://github.com/MuTsunTsai/gulp-5-test

In this repo, the gulp file is doing a very simple task of copying the package.json file to a new location. If gulp is the only dependency installed in this repo, this is done as quickly as about 400ms, but as the installed dependencies increases, it becomes slower and slower. In this repo it has a few dozens of dependencies installed (as in the case of my actual projects). If I use NPM to install them, after the installation Gulp executes the exact same task in about 1.2s, which is 3x slower than without other dependencies. If I use PNPM (which I usually do these days) instead, it's a lot even worse, taking over 6s just to complete this simple task.

Terminal output / screenshots

image

Please provide the following information:

  • OS & version [e.g. MacOS Catalina 10.15.4]: Windows 11 23H2
  • node version (run node -v): 20.10.0
  • npm version (run npm -v): 10.2.3
  • pnpm version: 8.15.5
  • gulp version (run gulp -v): CLI 3.0.0, local 5.0.0
@phated
Copy link
Member

phated commented Apr 1, 2024

This is due to our new directory walking technique. We're looking into ways to improve it in updates to glob-stream.

@MuTsunTsai
Copy link
Author

MuTsunTsai commented Apr 1, 2024

@phated Alright, thanks for the great work and I'm really happy to see v5 released in any case! Gulp has always been an irreplaceable part of my workflow.

Meanwhile, it appears that overriding the version of glob-stream fixes the issue for now (although I'm not sure if there's any compatibility issues with Gulp v5). For example, with PNPM:

{
	"pnpm": {
		"overrides": {
			"glob-stream": "7.0.0"
		}
	}
}

@MuTsunTsai
Copy link
Author

MuTsunTsai commented Apr 2, 2024

@phated So I was trying to understand how glob-stream works and see if there's anything I can contribute. My current understanding is that:

  • Everytime gulp.src() is called, it walks the entire directory tree looking for glob matches.

And the motivation of it is described in #118. Basically you guys are trying to make the behavior more predictable, so to speak.

I think to solve the performance issue, what is needed here is some kind of "partial matching check" mechanism, which is to see if a folder partially matches any glob (that is, if it is potentially possible for it to contain anything that will match), and if not, there's no need to look further into its subtree.

In my example, my glob is just the "package.json" file without any wildcard, and yet the walking part still go through the entire node_modules tree regardlessly (and that explains why PNPM is worse, as it creates a much larger folder tree than NPM). At the moment it visits the node_modules folder it should have concluded that "oh this is already deeper than the given glob, so it is impossible for it to contain what we want" and skipped.

I hope this makes sense to you. I'll see if I have more free time to contribute further.

@phated
Copy link
Member

phated commented Apr 3, 2024

@MuTsunTsai thanks! I'm thinking we'll do an optimization where we only traverse the glob-parent (or just a single file for singular globs)

@MuTsunTsai
Copy link
Author

Confirm fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants