Skip to content

Fix "Too many open files" if too small of a system limit is encountered #597

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
Merged
merged 2 commits into from
Nov 4, 2019
Merged

Fix "Too many open files" if too small of a system limit is encountered #597

merged 2 commits into from
Nov 4, 2019

Conversation

@vlm
Copy link
Contributor

@vlm vlm commented Nov 1, 2019 •
edited
Loading

  • The idea of this PR is that we can estimate the concurrency limits afforded by the system by examining its max open files limit. Based on that we set the number of lanes and the number of background tasks.
  • This PR requires Streamline file descriptor handling during spawning #596 for correctness, but there's no code interdependency.

rdar://55670543

@vlm vlm requested review from ddunbar and dmbryson as code owners November 1, 2019 00:05
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

1 similar comment
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

@vlm vlm force-pushed the small-file-limit branch from c6d85d8 to 92bccc8 Compare November 1, 2019 21:17
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

1 similar comment
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

@vlm vlm force-pushed the small-file-limit branch from 92bccc8 to accadb4 Compare November 1, 2019 21:32
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

1 similar comment
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

@vlm vlm force-pushed the small-file-limit branch from accadb4 to d4c6396 Compare November 1, 2019 21:36
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

3 similar comments
@vlm
Copy link
Contributor Author

vlm commented Nov 1, 2019

@swift-ci please smoke test

@vlm
Copy link
Contributor Author

vlm commented Nov 2, 2019

@swift-ci please smoke test

@vlm
Copy link
Contributor Author

vlm commented Nov 2, 2019

@swift-ci please smoke test

@vlm vlm force-pushed the small-file-limit branch from 4a6641b to 3900ed4 Compare November 2, 2019 03:52
@vlm
Copy link
Contributor Author

vlm commented Nov 2, 2019

@swift-ci please smoke test

1 similar comment
@vlm
Copy link
Contributor Author

vlm commented Nov 2, 2019

@swift-ci please smoke test

@vlm
Make tests work with different shells.
9e12eb4 
Lit uses `bash` if it can find it. Otherwise it uses /bin/sh.
Ninja uses strictly `/bin/sh` to run on unixes.
@vlm vlm force-pushed the small-file-limit branch from e0adf68 to 9e12eb4 Compare November 4, 2019 18:02
@vlm
Copy link
Contributor Author

vlm commented Nov 4, 2019

@swift-ci please smoke test

1 similar comment
@vlm
Copy link
Contributor Author

vlm commented Nov 4, 2019

@swift-ci please smoke test

@vlm vlm merged commit 972366b into swiftlang:master Nov 4, 2019
@vlm vlm deleted the small-file-limit branch November 4, 2019 19:21
@jakepetroules jakepetroules mentioned this pull request Sep 5, 2024
Closed
jakepetroules added a commit that referenced this pull request Sep 5, 2024
@jakepetroules
Improve build performance by a factor of $CORE_COUNT
722c903 
This fixes a regression introduced by #597, where the following line 3900ed4#diff-bd65cf12ed29b52c530ff04d2c678eb66883eddfcb44d5dbb8fe499f25865e93R219 presumably meant to use std::max instead of std::min.

The impact of this is that estimateTaskLimits falls back to a default case of returning only a single lane, regardless of the number of requested lanes, and consequently all builds are serialized.

Now, we simply return the value of _getmaxstdio directly. The documentation does not indicate that this function can fail or return a negative or zero value, and attempting to set the value with _setmaxstdio will terminate the process if given any value less than _IOB_ENTRIES (3).

Closes: #936
@jakepetroules jakepetroules mentioned this pull request Sep 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmbryson dmbryson dmbryson approved these changes

@ddunbar ddunbar Awaiting requested review from ddunbar

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vlm @dmbryson @jakepetroules