-
Notifications
You must be signed in to change notification settings - Fork 333
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
Very high bandwidth usage in monorepo #1351
Comments
Hi, thanks for reporting. In a single run of npm-check-updates results are memoized by dependency name. So in a monorepo you should see only one request per dependency, even if it is used in many packages. Note that you can enable caching across runs with the
|
Ah, thank you very much. My bad for not reading the docs. The thing is: I have packages with the exact same dependencies and it was still checking and requesting for each one. |
Sorry I misunderstood initially (edited). If you are seeing multiple requests for the same dependency, then we should investigate as that is not the expected behavior. It should memoize the results of every dependency across monorepo packages. If you have a simple reproduce case that would be helpful. If not, I'll do a test run in workspaces mode and print some logs to see if I can reproduce it. |
I can tell you the stats of mine after fetching 90 dependencies: Received Bytes: 80.039.784 The sent packets does roughly match the amount of dependencies I have, so there must be a problem with the amount of data these requests hold. I guess, a simple vite create command and creating a simple mono repo with 4 packages with these dependencies should be enough to trigger this: {
"devDependencies": {
"@types/node": "^20.9.2",
"@vitest/coverage-v8": "^0.34.6",
"@vitest/ui": "^0.34.6",
"eslint": "^8.54.0",
"tslib": "^2.6.2",
"typescript": "^5.2.2",
"vitest": "^0.34.6"
}
} I can't currently create a simple repro for that, sorry. I try to make one as fast as possible later on. |
Great, thanks. Can you also send the stats for |
Yup, here are the stats: Received Bytes: 7.069.026 It's basically downloading the entire package from the release tab. |
Also, yes, cache does help. Previously I had to cancel the ncu process, but now I tested it on a monorepo with way more packages: Received Bytes: 160.470.651 With Received Bytes: 33.774.245 |
I'm not sure what you mean by "release tab", but npm-check-updates does download the entire package manifest for each dependency. It seems possible to just fetch the dist-tags, which would be way less bandwidth, though we would lose the deprecation check (deprecated versions are currently excluded from ncu results by default).
Great, thanks. So in that case it could be a bug in the memoization. I will look into it. |
I was refering to this tab. Wouldn't it be possible to just read the first lines of a README or package.json or other manifest files? Is it possible to just request the first few chunks of the whole manifest file until a package version is found? Because 7.4 MB for just Typescript is way too much if you can just read it from smaller files of the repo. |
package.json is the only manifest file. The README does not typically contain the latest version (if it appears on a README, it's usually in the form of an SVG badge that is cached from the npm registry).
That's a great idea to stream it. Once #1329 gets merged, we should be able to use fetch.json.stream. |
Still its weird that the received packets are 7MB in total when the entire package.json file of typescript is just 3.44 KB. There must be a lot of unnecessary data fetched as well. |
Yes, good point. npm-check-updates currently uses pacote.packument which gets all manifests of all versions. That seems... bad 😅 |
Ah, so it essentially boils down to a faulty dependency. Well, I hope the new dependency reduces the bandwidth usage a lot. At least this issue gave us a little insight about this problem too. If you finally got this to work, try testing the bandwidth usage with AppNetworkCounter. It's free and doesn't even need an installation as it runs out of the box. I leave this issue open until the bandwidth is reduced in ncu 17. |
Basically yes. The use of other So there are three areas where the performance can be improved:
|
Let's start with the easy one. I've fixed #1 and published to |
Ok, this small change made this tool MUCH faster and reduced the bandwidth usage by a lot. Basically So the only issue is still the fetching of each dependency. |
Great, I'm glad that's fixed. I replaced |
Only the performance changed. TypeScript still costs 7MB and my repository still costs 33MB. The only thing that got reduced was the received packets with |
Hmmm. I would have expected that to change. I'll try it on a couple more test cases. |
There was a small error in my code that was preventing the manifest case. Please try |
Now, this made it worse:
Received Bytes: 14.133.320 My repo: Received Bytes: 67.151.019 You essentially doubled the downloading and also the requests got more. I guess instead of replacing, you just added it on top. |
Turns out it was a mistake with the arguments passed to Fixed in As for the download size, it turns out that https://github.com/npm/pacote/blob/9e9e18761dc2091f0030a9a4758a47ee3d8b11a4/lib/registry.js#L118 Disappointing, but it fits my understanding that the npm registry api does not have an additional endpoint for just the manifest. The real solution will be an abortable stream. |
Damn, but thanks for trying. Well, you managed to help me a lot with my monorepo, so I'm gonna wait for ncu 17 when you implemented the streams approach. |
Thanks for taking the time to report the issue. I learned some things! |
npm-check-updates
node >= 14.14
Steps to Reproduce
.ncurc:
Steps:
I've have a monorepo that has a main react package and some utility packages with the sames dependencies: @types/node, eslint, vitest and typescript.
I used AppNetworkCounter to monitor the bandwidth usage.
Current Behavior
My guess is, that this script re-checks every dependency even if it was already checked before. That is, if I have 4 packages in my monorepo each with
@types/node
, it checks it four times instead of caching it. As I wanted to update my monorepo, the bandwidth shot in the air with like 3MB/s and 1 or 2MB of requests and file reading for each dependency.Expected Behavior
The text was updated successfully, but these errors were encountered: