build: Enable building with Ninja #6780

ehsan · 2016-05-16T02:46:18Z

Checklist

tests and code linting passes
a test and/or benchmark is included
documentation is changed or added
the commit message follows commit guidelines

Affected core subsystem(s)

build

Description of change

Ninja is a build backend supported by gyp which is much faster than make
and is able to parallelize builds across all of the available cores very
well. On my machine, this reduces the average build time from 5:14
minutes to 4:33 minutes.

jbergstroem · 2016-05-16T03:23:22Z

@nodejs/build

MylesBorins · 2016-05-16T05:07:07Z

I know there is a guide that @Fishrock123 was working on.

rvagg · 2016-05-16T05:31:33Z

/cc @thlorenz

I didn't think we needed anything new to make it work with ninja?

bnoordhuis · 2016-05-16T05:52:47Z

For background/posterity, configure had a --ninja switch until 16 months ago. It was removed in de224d6 because the way it was wired up in the top-level Makefile was very inefficient.

That's not an issue when you run it as ninja -C out/Release but then you don't get the ./node symlink in the top-level directory.

Fishrock123 · 2016-05-16T14:54:23Z

The guide landed in core about two months ago: https://github.com/nodejs/node/blob/master/doc/guides/building-node-with-ninja.md

The above is a more reliable way of running Ninja, without having to interact with make.

On my machine, this reduces the average build time from 5:14
minutes to 4:33 minutes.

Sounds about right. If you really care about compile times, using ccache, even with make, will usually be drastically faster.

I'm not opposed to adding the configure flag for it, but you'll still need to be doing other stuff manually.

Fishrock123 · 2016-05-16T15:00:39Z

Actually, in the grand scheme of things, won't you still need to run tools.gyp_node.py manually?

If so... there's not much difference:

./configure --ninja
tools/gyp_node.py
ninja -C out/Release
ln -fs out/Release/node node

vs...

./configure
tools/gyp_node.py -f ninja
ninja -C out/Release
ln -fs out/Release/node node

ehsan · 2016-05-18T01:25:03Z

Actually, in the grand scheme of things, won't you still need to run tools.gyp_node.py manually?

No, configure runs it at the very end, so with this patch the second step would be unnecessary.

I'm wondering if it's possible to express creating the node symlink in the source tree root directory in gyp so that the last step would be unnecessary too? And continuing on that though train, things such as the test make target can also be expressed in gyp, so that it would work with both make and ninja as a target. But I'm digressing from the topic at hand!

FWIW, I'd be fine with just closing this PR, I was basically unaware of the guide you linked to. :)

Fishrock123 · 2016-05-18T16:44:40Z

Hey, I'd be all for removing a step -- this seems like a good idea to me.

ehsan · 2016-05-19T19:58:34Z

Hey, I'd be all for removing a step -- this seems like a good idea to me.

Great!

So what else should I do here? I tried to move symlinking srcdir/node into gyp but the fact that for debug builds we create node_g makes that super difficult, and I haven't found the correct way to do that in gyp yet.

Should I also update the guide for building with Ninja?

Fishrock123 · 2016-05-19T20:08:22Z

@ehsan I'd be ok with updating the guide here with just this for now, and the investigating the other things after.

Fishrock123 · 2016-05-19T20:08:58Z

@nodejs/build any objections even if the configure option doesn't work with make?

jbergstroem · 2016-05-19T20:46:16Z

@Fishrock123 seeing how the configure option refers to ninja I don't think a user should expect make to work. I guess the help text could refer to "build system" or similar to make it slightly more obvious.

ehsan · 2016-05-20T15:27:51Z

Updated the guide in the new version of the patch.

Fishrock123 · 2016-05-25T14:44:13Z

configure

+parser.add_option('--ninja',
+    action='store_true',
+    dest='use_ninja',
+    help='generate a Ninja based build system')


Sounds like it produces the build system itself, could we clarify this to be like the xcode one? (Also per @jbergstroem's comment)

Fishrock123 · 2016-05-25T14:44:34Z

LGTM minus a nit. :)

ehsan · 2016-05-25T16:48:48Z

@Fishrock123 I originally had the string message say "generate build files for use with Ninja" to mirror the xcode message, and changed it according to my understanding of @jbergstroem's comment. Can you please let me know what string you'd prefer me to use here? Thanks!

Fishrock123 · 2016-05-25T19:07:09Z

I think "generate build files for use with Ninja" should be fine. If it was the originally, oops. It was probably a reaction to my comment which wasn't well grounded anyways.

Ninja is a build backend supported by gyp which is much faster than make and is able to parallelize builds across all of the available cores very well. On my machine, this reduces the average build time from 5:14 minutes to 4:33 minutes.

ehsan · 2016-05-26T19:53:35Z

Addressed the comment in 55321ab.

Fishrock123 · 2016-05-27T22:12:29Z

LGTM

Ninja is a build backend supported by gyp which is much faster than Make and is able to parallelize builds across all of the available cores very well. On my machine, this reduces the average build time from 5:14 minutes to 4:33 minutes. Refs: nodejs#467 Refs: de224d6 PR-URL: nodejs#6780 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Fishrock123 · 2016-05-27T22:18:45Z

Thanks! Landed in de50202 with an updated commit message of build: re-add --ninja option to configure and fixed some lines in the commit description that were beyond 72 columns wide.

Ninja is a build backend supported by gyp which is much faster than Make and is able to parallelize builds across all of the available cores very well. On my machine, this reduces the average build time from 5:14 minutes to 4:33 minutes. Refs: nodejs#467 Refs: de224d6 PR-URL: nodejs#6780 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Ninja is a build backend supported by gyp which is much faster than Make and is able to parallelize builds across all of the available cores very well. On my machine, this reduces the average build time from 5:14 minutes to 4:33 minutes. Refs: #467 Refs: de224d6 PR-URL: #6780 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

MylesBorins · 2016-07-11T21:49:03Z

@Fishrock123 lts?

MylesBorins · 2016-08-30T18:45:22Z

ping @Fishrock123

Fishrock123 · 2016-09-04T20:48:50Z

lts could be good

Ninja is a build backend supported by gyp which is much faster than Make and is able to parallelize builds across all of the available cores very well. On my machine, this reduces the average build time from 5:14 minutes to 4:33 minutes. Refs: #467 Refs: de224d6 PR-URL: #6780 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

mscdex added the build Issues and PRs related to build files or the CI. label May 16, 2016

ehsan force-pushed the ninja branch from 0c1a972 to ddabc79 Compare May 20, 2016 15:27

Fishrock123 reviewed May 25, 2016
View reviewed changes

build: Enable building with Ninja

55321ab

Ninja is a build backend supported by gyp which is much faster than make and is able to parallelize builds across all of the available cores very well. On my machine, this reduces the average build time from 5:14 minutes to 4:33 minutes.

ehsan force-pushed the ninja branch from ddabc79 to 55321ab Compare May 26, 2016 19:53

Fishrock123 closed this May 27, 2016

MylesBorins mentioned this pull request Jul 11, 2016

Audit commits not found on v4.x-staging #7661

Closed

MylesBorins added the lts-watch-v4.x label Jul 11, 2016

MylesBorins added land-on-v4.x and removed lts-watch-v4.x labels Oct 10, 2016

MylesBorins mentioned this pull request Oct 26, 2016

V4.6.2 proposal #9298

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Enable building with Ninja #6780

build: Enable building with Ninja #6780

ehsan commented May 16, 2016 •

edited by jbergstroem

Loading

jbergstroem commented May 16, 2016

MylesBorins commented May 16, 2016

rvagg commented May 16, 2016

bnoordhuis commented May 16, 2016

Fishrock123 commented May 16, 2016

Fishrock123 commented May 16, 2016 •

edited

Loading

ehsan commented May 18, 2016

Fishrock123 commented May 18, 2016

ehsan commented May 19, 2016

Fishrock123 commented May 19, 2016

Fishrock123 commented May 19, 2016

jbergstroem commented May 19, 2016

ehsan commented May 20, 2016

Fishrock123 May 25, 2016

Fishrock123 commented May 25, 2016

ehsan commented May 25, 2016

Fishrock123 commented May 25, 2016

ehsan commented May 26, 2016

Fishrock123 commented May 27, 2016

Fishrock123 commented May 27, 2016 •

edited

Loading

MylesBorins commented Jul 11, 2016

MylesBorins commented Aug 30, 2016

Fishrock123 commented Sep 4, 2016

build: Enable building with Ninja #6780

build: Enable building with Ninja #6780

Conversation

ehsan commented May 16, 2016 • edited by jbergstroem Loading

Checklist

Affected core subsystem(s)

Description of change

jbergstroem commented May 16, 2016

MylesBorins commented May 16, 2016

rvagg commented May 16, 2016

bnoordhuis commented May 16, 2016

Fishrock123 commented May 16, 2016

Fishrock123 commented May 16, 2016 • edited Loading

ehsan commented May 18, 2016

Fishrock123 commented May 18, 2016

ehsan commented May 19, 2016

Fishrock123 commented May 19, 2016

Fishrock123 commented May 19, 2016

jbergstroem commented May 19, 2016

ehsan commented May 20, 2016

Fishrock123 May 25, 2016

Choose a reason for hiding this comment

Fishrock123 commented May 25, 2016

ehsan commented May 25, 2016

Fishrock123 commented May 25, 2016

ehsan commented May 26, 2016

Fishrock123 commented May 27, 2016

Fishrock123 commented May 27, 2016 • edited Loading

MylesBorins commented Jul 11, 2016

MylesBorins commented Aug 30, 2016

Fishrock123 commented Sep 4, 2016

ehsan commented May 16, 2016 •

edited by jbergstroem

Loading

Fishrock123 commented May 16, 2016 •

edited

Loading

Fishrock123 commented May 27, 2016 •

edited

Loading