Skip to content

set $NINJAFLAGS to make sure Ninja doesn't use all visible cores when building Qt5#2338

Merged
akesandgren merged 3 commits intoeasybuilders:developfrom
boegel:20210218120915_new_pr_EWJjRhxdvd
Mar 28, 2022
Merged

set $NINJAFLAGS to make sure Ninja doesn't use all visible cores when building Qt5#2338
akesandgren merged 3 commits intoeasybuilders:developfrom
boegel:20210218120915_new_pr_EWJjRhxdvd

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Feb 18, 2021 •
edited
Loading

(created using eb --new-pr)

fixes #2076

@boegel boegel changed the title set $NINJAFLAGS to make sure Ninja doesn't use all visible cores when building Qt5 (fixes #2076) set $NINJAFLAGS to make sure Ninja doesn't use all visible cores when building Qt5 Feb 18, 2021
@boegel boegel added the bug fix label Feb 18, 2021
@boegel boegel added this to the 4.3.3 (next release) milestone Feb 18, 2021
@branfosj
Copy link
Copy Markdown
Member

branfosj commented Feb 18, 2021

Test report by @branfosj

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.14.2-GCCcore-10.2.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
bear-pg0211u17a.bear.cluster - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6248 CPU @ 2.50GHz (cascadelake), Python 3.6.8
See https://gist.github.com/791b4cd9650eef20d139873cd0d20ebe for a full test report.

@branfosj
Copy link
Copy Markdown
Member

branfosj commented Feb 18, 2021

Test report by @branfosj

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.13.1-GCCcore-8.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
bear-pg0211u18a.bear.cluster - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6248 CPU @ 2.50GHz (cascadelake), Python 3.6.8
See https://gist.github.com/a14974e2a68ecbf2bdd7f6c30e3f2763 for a full test report.

@branfosj
Copy link
Copy Markdown
Member

branfosj commented Feb 18, 2021

Test report by @branfosj

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.14.1-GCCcore-9.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
bear-pg0211u17b.bear.cluster - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6248 CPU @ 2.50GHz (cascadelake), Python 3.6.8
See https://gist.github.com/aed794b5ce7052e8161a4109a8488435 for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 18, 2021

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.14.2-GCCcore-10.2.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3551.doduo.os - Linux RHEL 8.2, x86_64, AMD EPYC 7552 48-Core Processor (zen2), Python 3.6.8
See https://gist.github.com/c01e96c444f316466995bef2b6d64270 for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 18, 2021

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.10.1-foss-2018b.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3117.skitty.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/9b2ca0234611b964310506efe567438a for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 18, 2021

@branfosj suggested that we should find a more general place to add -j to $NINJAFLAGS, perhaps even in EasyBuild framework, since it likely also affects other software.

Just doing so in the MesonNinja generic easyblock is not good enough, because for Qt5 specifically the use of ninja is indirect: the EB_Qt easyblock we use for building Qt5 is based on ConfigureMake...

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 18, 2021

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.13.1-GCCcore-8.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node2610.swalot.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz (haswell), Python 3.6.8
See https://gist.github.com/de5e26656449b2c35cfa131c244d3b34 for a full test report.

akesandgren
akesandgren reviewed May 24, 2021
View reviewed changes
Comment thread easybuild/easyblocks/q/qt.py
# so $NINJAFLAGS is set to control number of parallel processes used by Ninja
if LooseVersion(self.version) >= LooseVersion('5'):
if get_software_root('Ninja'):
env.setvar('NINJAFLAGS', '-j%s' % self.cfg['parallel'])
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren May 24, 2021

Choose a reason for hiding this comment

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

It's likely a bad idea to overwrite NINJAFLAGS like this. User could have set it for some reason. And it should check if "-j" is already in NINJAFLAGS and not override it if it is...

Copy link
Copy Markdown
Contributor

@surak surak Jul 2, 2021

Choose a reason for hiding this comment

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

Should one add this variable on ninja itself? Or make ninja respect parallel?

Copy link
Copy Markdown
Member Author

@boegel boegel Mar 25, 2022

Choose a reason for hiding this comment

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

I've just figured out that $NINJAFLAGS is very specific to Qt5, so in that case I'd argue that ignoring $NINJAFLAGS if it's set already actually makes total sense (you don't want a random value for a pretty generically named environment variable like $NINJAFLAGS to mess up your Qt5 build).

@boegel boegel modified the milestones: 4.4.0, release after 4.4.0 May 26, 2021
@branfosj branfosj mentioned this pull request May 29, 2021
Merged
@boegel boegel modified the milestones: 4.4.2, release after 4.4.2 Sep 1, 2021
@boegel boegel modified the milestones: 4.5.1, release after 4.5.1 Dec 7, 2021
@boegel boegel modified the milestones: 4.5.2, release after 4.5.2 Jan 14, 2022
@boegel boegel removed this from the 4.5.3 milestone Feb 9, 2022
@boegel boegel added this to the release after 4.5.3 milestone Feb 9, 2022
@akesandgren
Copy link
Copy Markdown
Contributor

akesandgren commented Feb 14, 2022

This one could use a bit of revisiting from @boegel

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 16, 2022

@akesandgren We should probably do this on the framework side, not just for Qt5...

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 25, 2022 •
edited
Loading

I did a bit more digging, and apparently $NINJAFLAGS is not a generic thing for Ninja, it's very specific to Qt5...
Ninja itself only picks up a single environment variable according to https://ninja-build.org/manual.html, and that's only to tweak how it reports status; see also the discussion in ninja-build/ninja#797

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 25, 2022

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.10.1-foss-2018a.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3164.skitty.os - Linux CentOS Linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/b66ca12cd455478421e78e45cd0bd22f for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 26, 2022

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.8.0-foss-2017b.eb
  • SUCCESS Qt5-5.9.3-foss-2017b.eb
  • SUCCESS Qt5-5.6.0-foss-2016a.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
node2622.swalot.os - Linux CentOS Linux 7.9.2009, x86_64, Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz (haswell), Python 3.6.8
See https://gist.github.com/2e4eca92bc73a706382a73aa8ae1eef8 for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 26, 2022

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Qt5-5.13.1-GCCcore-8.3.0.eb
  • SUCCESS Qt5-5.14.1-GCCcore-9.3.0.eb
  • SUCCESS Qt5-5.14.2-GCCcore-10.2.0.eb
  • SUCCESS Qt5-5.15.2-GCCcore-10.3.0.eb
  • SUCCESS Qt5-5.15.2-GCCcore-11.2.0.eb

Build succeeded for 5 out of 5 (5 easyconfigs in total)
node3575.doduo.os - Linux RHEL 8.4, x86_64, AMD EPYC 7552 48-Core Processor (zen2), Python 3.6.8
See https://gist.github.com/d07ce43175b3de250035daf99ea4d547 for a full test report.

akesandgren
akesandgren approved these changes Mar 28, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Copy Markdown
Contributor

akesandgren commented Mar 28, 2022

Going in, thanks @boegel!

@akesandgren akesandgren merged commit 56f46ee into easybuilders:develop Mar 28, 2022
@boegel boegel deleted the 20210218120915_new_pr_EWJjRhxdvd branch March 28, 2022 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akesandgren akesandgren akesandgren approved these changes

+1 more reviewer

@surak surak surak left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

4.5.4

Development

Successfully merging this pull request may close these issues.

Qt Make/Ninja interaction

4 participants

@boegel @branfosj @akesandgren @surak