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

Number of processes used by 'make' should not be specified in documentation #8286

Closed
thecoolestguy opened this issue Aug 26, 2016 · 11 comments
Closed
Assignees
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.

Comments

@thecoolestguy
Copy link
Contributor

  • v6.4.0
  • Darwin 14.5.0
  • doc

In several of the documentation files, the number of simultaneous jobs is specified by the -j option and the number varies between files. According to the 'make' man page, "If the -j option is given without an argument, make will not limit the number of jobs that can run simultaneously." The documentation should just have a '-j' without a number, so as to remain system-agnostic.

The files in which this issue resides are as follows:

https://github.com/nodejs/node/blob/master/doc/onboarding.md
https://github.com/nodejs/node/blob/master/doc/onboarding-extras.md
https://github.com/nodejs/node/blob/master/doc/guides/building-node-with-ninja.md
https://github.com/nodejs/node/blob/master/CONTRIBUTING.md
https://github.com/nodejs/node/blob/master/.github/PULL_REQUEST_TEMPLATE.md

@mscdex mscdex added doc Issues and PRs related to the documentations. build Issues and PRs related to build files or the CI. labels Aug 26, 2016
thecoolestguy added a commit to thecoolestguy/node that referenced this issue Aug 26, 2016
Removing specified number of jobs from -j option in the documentation
steps for running the make command. A '-j' without a number specified
will use the maximum number allowed for any given machine.
Fixes: nodejs#8286
@Trott
Copy link
Member

Trott commented Aug 26, 2016

Running make clean && make distclean && ./configure && make -j puts the load on my machine way up...it peaked in the high 200s, and then build failed with:

make[1]: *** wait: No child processes.  Stop.

Probably best bet is to just standardize everything on make -j8. Smallest changeset too. Just one file.

@addaleax
Copy link
Member

According to the 'make' man page, "If the -j option is given without an argument, make will not limit the number of jobs that can run simultaneously."

That is a very good reason not to recommend -j – when hundreds of tasks can run in parallel, make will allow that with -j, as @Trott observed. I’d be surprised if there were any circumstances under which that would not either crash or freeze the system? 😄

make -j8 sounds fine, people who know what that “magic” 8 means could still use something else.

@jasnell
Copy link
Member

jasnell commented Aug 27, 2016

I always turn my -j up to 11.

@silverwind
Copy link
Contributor

I'd rather not recommend -j at all.

BTW: Wouldn't something like make -j $(nproc) be ideal in most cases? (Linux only)

@Trott
Copy link
Member

Trott commented Aug 27, 2016

I think any of the following would be a slight improvement over the current situation:

  • standardize all docs on -j8
  • standardize all docs on -j4
  • standardize all docs to not mentioning -j option; advanced users will already know about it and what to do with it
  • standardize all docs to not use -j in initial examples but to sometimes include an optional explanation (one or two sentences) of the -j option where it makes sense; for example, it probably makes sense in the BUILDING.md doc; it probably is OK to omit in the .github template where brevity is important

@Fishrock123
Copy link
Contributor

Fishrock123 commented Aug 28, 2016

I think we should standardize -j4. Easy enough on most laptops, and means newcomers have a better experience. Anyone who knows more about it can adjust it to their liking.

(Note: I use -j8. I also have an i7 processor.)

@mhdawson
Copy link
Member

We had problems in the release CI where -j was specified, causing failures due to the high load so we definitely don't want -j. What we really want is -j $(NUM_CPUS) but that might be a bit wordy to explain properly.

@bnoordhuis
Copy link
Member

On most UNIX-y systems you can write make -j$(getconf _NPROCESSORS_ONLN). Still a bit wordy though.

@Trott
Copy link
Member

Trott commented Aug 30, 2016

@thecoolestguy It seems like there are a lot of options here, none of which is the clear frontrunner for consensus. However, all seem better than the current situation.

I'd recommend reading the discussion here, picking the option you are most convinced of, and updating your pull request appropriately. While I have my preference, I can live with any of the alternatives discussed and I'd rather see one land than have it stall out due to a lack of clear consensus.

I suspect others will feel the same way as the one thing we do have consensus about is that the current docs about make options can be improved.

@Fishrock123
Copy link
Contributor

#9961 Should fix this.

@Trott
Copy link
Member

Trott commented Dec 5, 2016

I'm going to close this as stalled, but feel free to comment or re-open if anyone thinks that's not the right thing to do.

@Trott Trott closed this as completed Dec 5, 2016
Fishrock123 pushed a commit that referenced this issue Dec 9, 2016
Adds a note to the BUILDING doc to encourage parallelizing make. When I
first built node I didn't know this trick and thought that the build was
just stuck in an infinite loop after waiting for 10 minutes.

Refs: #8286
Refs: #9881

PR-URL: #9961
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Fishrock123 pushed a commit that referenced this issue Dec 13, 2016
Adds a note to the BUILDING doc to encourage parallelizing make. When I
first built node I didn't know this trick and thought that the build was
just stuck in an infinite loop after waiting for 10 minutes.

Refs: #8286
Refs: #9881

PR-URL: #9961
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 16, 2017
Adds a note to the BUILDING doc to encourage parallelizing make. When I
first built node I didn't know this trick and thought that the build was
just stuck in an infinite loop after waiting for 10 minutes.

Refs: #8286
Refs: #9881

PR-URL: #9961
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
Adds a note to the BUILDING doc to encourage parallelizing make. When I
first built node I didn't know this trick and thought that the build was
just stuck in an infinite loop after waiting for 10 minutes.

Refs: #8286
Refs: #9881

PR-URL: #9961
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
Adds a note to the BUILDING doc to encourage parallelizing make. When I
first built node I didn't know this trick and thought that the build was
just stuck in an infinite loop after waiting for 10 minutes.

Refs: #8286
Refs: #9881

PR-URL: #9961
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants