Skip to content
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

tools/check: increase the parallelism for building test binaries #48985

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

bb7133
Copy link
Member

@bb7133 bb7133 commented Nov 28, 2023

What problem does this PR solve?

Issue Number: ref #31880

Problem Summary:

What changed and how does it work?

Increase the parallelism when building the test binaries, by default it is the number of cores(https://pkg.go.dev/cmd/go#hdr-Compile_packages_and_dependencies):

-p n
the number of programs, such as build commands or
test binaries, that can be run in parallel.
The default is GOMAXPROCS, normally the number of CPUs available.

I found that when running the 'building binaries stage' of make ut, the CPU usage is not high with the default value. So I think it's a better idea to increase the parallelism. In this PR, I doubled it.

The result of make ut time in my Macbook M2 Pro:

(Before this PR)

bash> make ut
...
building task finish, maxproc=10, count=4822, takes=12m32.547299958s 
...

(After this PR)

bash> make ut
...
building task finish, parallelism=20, count=4822, takes=6m45.4092545s

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.
      This is a change of the test framework, it does not change any behavior of the release, and it is trivial enough.

Side effects

  • None

Documentation

  • None

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Merging #48985 (3de6266) into master (68271e9) will increase coverage by 1.4770%.
Report is 14 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #48985        +/-   ##
================================================
+ Coverage   70.9767%   72.4537%   +1.4770%     
================================================
  Files          1368       1391        +23     
  Lines        404407     412082      +7675     
================================================
+ Hits         287035     298569     +11534     
+ Misses        97376      94672      -2704     
+ Partials      19996      18841      -1155     
Flag Coverage Δ
integration 43.6858% <ø> (?)
unit 70.9767% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 53.9663% <ø> (ø)
parser ∅ <ø> (∅)
br 48.3370% <ø> (-4.6887%) ⬇️

@@ -448,6 +450,8 @@ func main() {

// Get the correct count of CPU if it's in docker.
p = runtime.GOMAXPROCS(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work correctly in container environments?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I'm not sure, but as the old code it should be fine.

Copy link

ti-chi-bot bot commented Nov 30, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-11-29 07:55:56.942328092 +0000 UTC m=+995785.607554288: ☑️ agreed by tiancaiamao.
  • 2023-11-30 01:38:11.671465249 +0000 UTC m=+1059520.336691444: ☑️ agreed by Defined2014.

Copy link

ti-chi-bot bot commented Nov 30, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Defined2014, hawkingrei, tiancaiamao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Defined2014,hawkingrei,tiancaiamao]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot merged commit 86a7ab3 into pingcap:master Nov 30, 2023
16 checks passed
@bb7133 bb7133 deleted the bb7133/buildp branch November 30, 2023 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm release-note-none size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants