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

Remove hard-coded -j8 argument for make #610

Closed
julianoes opened this issue Nov 26, 2018 · 10 comments
Closed

Remove hard-coded -j8 argument for make #610

julianoes opened this issue Nov 26, 2018 · 10 comments
Labels
beginner A good issue for someone that only gets started core

Comments

@julianoes
Copy link
Collaborator

The current Makefile is sort of a wrapper script around cmake and that's a bit confusing.

Especially annoying is actually the hard-coded -j8 argument for make. It would be better if this was taken from the CPU count or your settings for make (MAKEFLAGS).

https://github.com/Dronecode/DronecodeSDK/blob/ef45e4ddb7a8c5dcd0ac20df9a330afdaa9a7be6/Makefile#L22

More thoughts: https://stackoverflow.com/questions/10567890/parallel-make-set-j8-as-the-default-option

@julianoes julianoes added core beginner A good issue for someone that only gets started labels Nov 26, 2018
@simrankedia
Copy link
Contributor

simrankedia commented Dec 2, 2018

Hi,
I would like to start working on this. Please guide me as I am new to open source contribution. I specifically want to know the following:

  1. What OS will be used for the build?
  2. How many cores should be used for the build, given total number of cores are n.

@julianoes
Copy link
Collaborator Author

Hi @simrankedia,

  1. The Makefile is used on Linux and macOS. For Windows we directly use cmake and don't use this Makefile helper.
  2. I usually build with n if the CPU has n cores/hyperthreads.

@simrankedia
Copy link
Contributor

simrankedia commented Dec 3, 2018

Hi @julianoes,
Thanks for the prompt reply. Just a clarification regarding point 2 above:
The machine is supposed to be doing nothing else while the build is going on? (trying to understand if we should use less than n cores for the build.)

Also, please let me know how to build all the required components, so that I can test the edit I have made.

@julianoes
Copy link
Collaborator Author

The machine is supposed to be doing nothing else while the build is going on? (trying to understand if we should use less than n cores for the build.)

You can still do other things but they will be slowed down. I think it's common practice to use -jX for X hypercores.

Check the build instructions here:
https://sdk.dronecode.org/en/contributing/build.html

@JonasVautherin
Copy link
Collaborator

You can still do other things but they will be slowed down. I think it's common practice to use -jX for X hypercores.

Agreed. And there is always nice that does process scheduling if you don't want it to take too many resources.

@simrankedia
Copy link
Contributor

Hi @julianoes
I am done with the changes. However, looks like I don't have the permission to create a branch. Please guide me on how to submit the changes for review.

Thanks.

@julianoes
Copy link
Collaborator Author

Nice @simrankedia! So the way it works is that you fork this repository and then create a pull request from there.
You can find some instructions here: https://akrabat.com/the-beginners-guide-to-contributing-to-a-github-project/

simrankedia added a commit to simrankedia/DronecodeSDK that referenced this issue Dec 9, 2018
@simrankedia
Copy link
Contributor

Hi @julianoes I have raised a pull request. Kindly have a look.

@JonasVautherin
Copy link
Collaborator

@simrankedia: I see your branch here on your fork, but not a PR. Are you sure you opened a pull request to this repo? From your branch, click "Pull Request", add a description (add "Resolves #610" in the description), and hit "Create pull request".

Then it will appear here on the DronecodeSDK repo :-).

@simrankedia
Copy link
Contributor

Hi @JonasVautherin, Sorry my bad. I have raised the PR now.

simrankedia added a commit to simrankedia/DronecodeSDK that referenced this issue Dec 10, 2018
julianoes added a commit that referenced this issue Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner A good issue for someone that only gets started core
Projects
None yet
Development

No branches or pull requests

3 participants