Skip to content

increase parallelism in selected artifact and distribtest jobs#28227

Merged
jtattermusch merged 1 commit intogrpc:masterfrom
jtattermusch:artifact_distribtest_32core
Dec 1, 2021
Merged

increase parallelism in selected artifact and distribtest jobs#28227
jtattermusch merged 1 commit intogrpc:masterfrom
jtattermusch:artifact_distribtest_32core

Conversation

@jtattermusch
Copy link
Copy Markdown
Contributor

@jtattermusch jtattermusch commented Nov 30, 2021

Followup for internal cl/413151606 (switched selected jobs to 32 core workers).

@jtattermusch jtattermusch added the release notes: no Indicates if PR should not be in release notes label Nov 30, 2021
lidizheng
lidizheng approved these changes Nov 30, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

LGTM. This PR along with the CL improves build time as mentioned.

The mapping from 12 jobs to 32 cores may need more thoughts. In the worst case, each Python build could spawn 32 threads?

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Nov 30, 2021

LGTM. This PR along with the CL improves build time as mentioned.

The mapping from 12 jobs to 32 cores may need more thoughts. In the worst case, each Python build could spawn 32 threads?

I think for that we'd have to set the number of parallel compiler jobs very high and I don't think we're doing that:

grpc/tools/run_tests/artifacts/build_artifact_python.sh

Line 42 in 74d0e39

export GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS=${GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS:-2}

(IIRC we're only setting GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS to 2 or 4 or something relatively low).

Oh ok, maybe here, but that's not for artifact build?

grpc/tools/run_tests/helper_scripts/build_python.sh

Line 125 in 25f5399

DEFAULT_PARALLEL_JOBS=$(nproc) || DEFAULT_PARALLEL_JOBS=4

(I think using nproc parallel jobs is an overkill and I'd be surprised if it lead to actual improvement in build time)

@lidizheng
Copy link
Copy Markdown
Contributor

lidizheng commented Nov 30, 2021

nproc is good for debugging, while fixed parallelism number is good for CI planning. I agree that the latter seems better for our use case.

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Dec 1, 2021

Timing improvements:
grpc_build_artifacts
BEFORE ~3h AFTER 1h50min

grpc_distribtests_python
BEFORE 130min AFTER 75min.

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Dec 1, 2021

Known failures:

@jtattermusch jtattermusch merged commit 95cbcd8 into grpc:master Dec 1, 2021
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Dec 2, 2021
@lidizheng lidizheng mentioned this pull request Dec 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@lidizheng lidizheng lidizheng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lidizheng lidizheng

Labels

bloat/none imported Specifies if the PR has been imported to the internal repository perf-change/none release notes: no Indicates if PR should not be in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jtattermusch @lidizheng @grpc-kokoro