Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Feb 18, 2021

(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
Member

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
Member

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
Member

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
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
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
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
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.

# 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
Contributor

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
Contributor

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
Member Author

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 added this to the release after 4.5.3 milestone Feb 9, 2022
@akesandgren
Copy link
Contributor

This one could use a bit of revisiting from @boegel

@boegel
Copy link
Member Author

boegel commented Feb 16, 2022

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

@boegel
Copy link
Member Author

boegel commented Mar 25, 2022

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
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
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
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.

Copy link
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
Contributor

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Qt Make/Ninja interaction

4 participants