fix: too many open files while creating a workspace #804
Conversation
|
To view this pull requests documentation preview, visit the following URL: Documentation is deployed and generated using docs.page. |
|
|
|
Hey @spydon, the CI workflows are on hold for some reason |
Ah, it was because of the rebase and that you haven't contributed before. Started them now. :) |
Head branch was pushed to by a user without write access
mahmuttaskiran
force-pushed
the
fix/too-many-open-files
branch
from
eb92fe4 to
ff85492
Compare
|
There are some tests failing which I couldn't relate with changes introduced with this PR |
|
It's the new Flutter version that breaks the tests, as can be seen here: #806 |
I plan to update tests this weekend. Thank you! |
| if (packageMap.containsKey(name)) { | ||
| throw MelosConfigException( | ||
| ''' | ||
| await Stream.fromIterable(pubspecFiles).parallel((pubspecFile) async { |
There was a problem hiding this comment.
I just realized that we should have a higher number here than the number of cores, since this isn't running on isolates. I don't know what a fitting number would be though, maybe try a few different and see how it affects performance?
There was a problem hiding this comment.
For context, the project has 294 packages. I've run time melos list three times for each case below (default, 25, 50, and 100) and didn't see much improvement. The higher numbers will increase the risk of "too many open files" issue
Here are some benchmarks when parallelism:
- was null (number of cores by default which is 10 on my device):
melos list 1.27s user 0.68s system 89% cpu 2.179 total
melos list 1.31s user 0.71s system 92% cpu 2.164 total
melos list 1.32s user 0.77s system 90% cpu 2.311 total
- was 25:
melos list 1.32s user 0.72s system 68% cpu 2.968 total
melos list 1.31s user 0.74s system 89% cpu 2.285 total
melos list 1.28s user 0.72s system 91% cpu 2.204 total
- was 50:
melos list 1.32s user 0.72s system 88% cpu 2.312 total
melos list 1.30s user 0.74s system 86% cpu 2.351 total
melos list 1.31s user 0.75s system 88% cpu 2.341 total
- was 100:
melos list 1.32s user 0.74s system 86% cpu 2.375 total
melos list 1.30s user 0.73s system 91% cpu 2.220 total
melos list 1.31s user 0.73s system 89% cpu 2.290 total
There was a problem hiding this comment.
Just out of curiosity, what does it say with 1?
There was a problem hiding this comment.
The first three run:
melos list 1.27s user 0.75s system 54% cpu 3.752 total
melos list 1.26s user 0.70s system 90% cpu 2.161 total
melos list 1.29s user 0.72s system 90% cpu 2.212 total
Interesting :)
There was a problem hiding this comment.
Seems like we don't need parallelism then 😄
Description
Fixing #803
I was thinking of creating a new Future extension to handle parallelism, but then I discovered that Stream.parallel already does the job by using the number of processors for concurrency. It turned out to be a perfect fit for what I needed
Type of Change
feat-- New feature (non-breaking change which adds functionality)fix-- Bug fix (non-breaking change which fixes an issue)!-- Breaking change (fix or feature that would cause existing functionality to change)refactor-- Code refactorci-- Build configuration changedocs-- Documentationchore-- Chore