Skip to content

swift_build_support/cmake.py: parallelize default CMake build #66212

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
May 30, 2023
Merged

swift_build_support/cmake.py: parallelize default CMake build #66212

merged 2 commits into from
May 30, 2023

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented May 30, 2023 •
edited
Loading

CMake's bootstrapping script can't infer a number of parallel build jobs from the number of CPU cores on its own, which means that it's not parallelized by default. Let's get the value of CPU cores when build_jobs argument is not explicitly set.

@MaxDesiatov
swift_build_support/cmake.py: parallelize default CMake build
b427e91 
CMake's bootstrapping script can't infer a number of parallel build jobs from the number of CPU cores on its own, which means that it's not parallelized by default. Let's get the value of CPU cores when `build_jobs` is not explicitly set.
@MaxDesiatov MaxDesiatov added build-script Area → utils: The build script cmake labels May 30, 2023
@MaxDesiatov MaxDesiatov self-assigned this May 30, 2023
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

compnerd
compnerd approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea to me!

I think that using .format would be nicer to be more modern python but 🤷‍♂️ that is a nit.

@MaxDesiatov
Copy link
Contributor Author

For modern Python I'd prefer an f-string, which seems to me like a thing closest to Swift string interpolation, but I don't think we started requiring specific minor Python versions yet, right? I assume we have to support Python 3.0 in these scripts?

@compnerd
Copy link
Member

Pretty sure that we want Python 3.9 as that is what Xcode and Visual Studio already ship with. The question is, do the oldest Linux distributions that we support have that python version?

@MaxDesiatov MaxDesiatov requested a review from shahmishal May 30, 2023 15:12
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@shahmishal
Copy link
Member

@swift-ci python lint

etcwilde
etcwilde approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. I'm someone curious how much improvement we get here because make -j should impose no limit on the number of jobs running simultaneously. Were we over-provisioning jobs? Anyway, seems reasonable.

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented May 30, 2023 •
edited
Loading

make -j only has an effect on CMake's post-bootstrap build jobs, while a substantial amount of time was spent building code sequentially in CMake's bootstrap script without that --parallelize option, which is called on a line preceding the make -j invocation.

@MaxDesiatov MaxDesiatov merged commit bbfd410 into main May 30, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/parallel-cmake-bootstrap branch May 30, 2023 21:29
meg-gupta pushed a commit to meg-gupta/swift that referenced this pull request Jun 2, 2023
@MaxDesiatov @meg-gupta
swift_build_support/cmake.py: parallelize default CMake build (swiftl…
281acac 
…ang#66212)

CMake's bootstrapping script can't infer a number of parallel build jobs from the number of CPU cores on its own, which means that it's not parallelized by default. Let's get the value of CPU cores when `build_jobs` argument is not explicitly set.

`make -j` only has an effect on CMake's post-bootstrap build jobs, while a substantial amount of time was spent building code sequentially in CMake's bootstrap script without that `--parallelize` option, which is called on a line preceding the `make -j` invocation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd approved these changes

@etcwilde etcwilde etcwilde approved these changes

@shahmishal shahmishal shahmishal approved these changes

@al45tair al45tair Awaiting requested review from al45tair

Assignees

@MaxDesiatov MaxDesiatov

Labels
build-script Area → utils: The build script cmake
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MaxDesiatov @compnerd @shahmishal @etcwilde