CI: better parallelism for functionality-test build #1805
Conversation
.github/workflows/default.yaml
Outdated
| - run: | | ||
| sudo make install | ||
| sudo make -j`nproc` install |
There was a problem hiding this comment.
This change request assumes that old "make [all]" command was restored and this line is specific to Squid installation.
I am not violently against adding -jn to "make install", but it feels wrong (unless it buys us significantly faster builds) because most folks are not going to use this kind of installation command and because errors from parallel installs are more difficult to interpret. In other words, I think this optimization needs to be justified.
| @@ -44,9 +44,8 @@ jobs: | |||
|
|
|||
| - run: ./bootstrap.sh | |||
| - run: ./configure --with-openssl | |||
| - run: make -j2 | |||
There was a problem hiding this comment.
Is there a better way to trigger parallel builds than remembering to specify -j... every time we ran "make"? Perhaps we should set MAKEFLAGS or use some kind of autoconf/automake tricks?
There was a problem hiding this comment.
I think environment variables are even worse than -jarg as they are not explicitly visible, are sticky and might be not propagated in case of exec or containerized runs
There was a problem hiding this comment.
Just -j with not specific N value may be what you are after.
There was a problem hiding this comment.
AFAIK "-j" with no numeric argument means "use as many parallel processes as you want". This doesn't sound a good idea in a shared environment where resources (especially core memory) might be constrained. Am I missing anything?
There was a problem hiding this comment.
I share Francesco's concern. FWIW, IIRC, in some environments I have tested (with more than n=2 CPU cores), -j was noticeably slower than -jn.
I am not going to object to bare -j if others prefer that but please make this decision based on GitHub Actions CI tests.
rousskov
changed the title
rousskov
added
the
S-waiting-for-author
kinkie
changed the title
kinkie
added
S-waiting-for-reviewer
| @@ -44,9 +44,8 @@ jobs: | |||
|
|
|||
| - run: ./bootstrap.sh | |||
| - run: ./configure --with-openssl | |||
| - run: make -j2 | |||
There was a problem hiding this comment.
I share Francesco's concern. FWIW, IIRC, in some environments I have tested (with more than n=2 CPU cores), -j was noticeably slower than -jn.
I am not going to object to bare -j if others prefer that but please make this decision based on GitHub Actions CI tests.
rousskov
added
S-waiting-for-author
|
I have adjusted PR description to detail this change rationale and the source of the original n=2 value. |
|
Notice: in practice this might or might not help on average, but it will ensure we make use of whatever resources Github will give to the worker we are allocated when we are. Landing this |
kinkie
added
M-cleared-for-merge
Use nproc to detect the actual number of CPU cores available instead of going by an old "2 cores" value from GitHub Actions runners docs.
squid-anubis
added
the
M-waiting-staging-checks
squid-anubis
added
M-merged
Use nproc to detect the actual number of CPU cores available instead of going by an old "2 cores" value from GitHub Actions runners docs.
Use nproc to detect the actual number of CPU cores available instead of
going by an old "2 cores" value from GitHub Actions runners docs.