Scons: provide the number of logical processors as the number of maximum concurrent jobs to speed up builds

[LeonarddeR]

Link to issue number:

None

Summary of the issue:

Building NVDA can take some time. SCons has the ability to provide the number of concurrent jobs during build.

Description of how this pull request fixes the issue:

Specify the maximum number of concurrent jobs when building NVDA.

Testing strategy:

Tested locally on an Intel Core i7-9850H using PowerShell's measure-command { .\scons.bat source }, ensuring the venv was created prerun.

• Without the change, 170 seconds
• With the change, 59 seconds

Known issues with pull request:

Output sometimes get a bit scrambled, i.e. the output of the cl command comes before SCons output the name of the file being build.

Change log entries:

For Developers

• Set up SCons to build with multiple concurrent jobs, equal to the number of logical processors in the system (i.e. 8 on a quat core Intel CPU with hyper threading). This can dramatically decrease build times on multi core systems.

Code Review Checklist:

• Pull Request description:
  • description is up to date
  • change log entries
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• API is compatible with existing add-ons.
• Documentation:
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English

[LeonarddeR]

Note, this doesn't seem to have any noticeable benefit on appveyor.

[josephsl]

Hi, probably because it uses virtual machine instances, which can impact processor performance. Thanks.

[Brian1Gaff]

So if that failed would the correct errors still occur, Not thatI would be doing this, but taking up the whole of the processor in this way is bound to slug bits of it? Brian ***@***.*** Sent via blueyonder. Please address personal E-mail to:- ***@***.***, putting 'Brian Gaff' in the display name field. Newsgroup monitored: alt.comp.blind-users ----- Original Message ----- From: "Leonard de Ruijter" ***@***.***> To: "nvaccess/nvda" ***@***.***> Cc: "Subscribed" ***@***.***> Sent: Monday, January 10, 2022 6:50 PM Subject: [nvaccess/nvda] Scons: provide the number of logical processors as the number of maximum concurrent jobs to speed up builds (PR #13226)

…

<!-- Please read and fill in the following template, for an explanation of the sections see: https://github.com/nvaccess/nvda/blob/master/devDocs/githubPullRequestTemplateExplanationAndExamples.md Please also note that the NVDA project has a Citizen and Contributor Code of Conduct which can be found at https://github.com/nvaccess/nvda/blob/master/CODE_OF_CONDUCT.md. NV Access expects that all contributors and other community members read and abide by the rules set out in this document while participating or contributing to this project. This includes creating or commenting on issues and pull requests. Please initially open PRs as a draft. See https://github.com/nvaccess/nvda/wiki/Contributing --> ### Link to issue number: None ### Summary of the issue: Building NVDA can take some time. SCons has the ability to provide the number of concurrent jobs during build. ### Description of how this pull request fixes the issue: Specify the maximum number of concurrent jobs when building NVDA. ### Testing strategy: Tested locally on an Intel Core i7-9850H using PowerShell's `measure-command { .\scons.bat source }`, ensuring the venv was created prerun. * Without the change, 170 seconds * With the change, 59 seconds ### Known issues with pull request: Output sometimes get a bit scrambled, i.e. the output of the cl command comes before SCons output the name of the file being build. ### Change log entries: For Developers - Set up SCons to build with multiple concurrent jobs, equal to the number of logical processors in the system (i.e. 8 on a quat core Intel CPU with hyper threading). This can dramatically decrease build times on multi core systems. ### Code Review Checklist: <!-- This checklist is a reminder of things commonly forgotten in a new PR. Authors, please do a self-review of this pull-request. Check items to confirm you have thought about the relevance of the item. Where items are missing (eg unit / system tests), please explain in the PR. To check an item `- [ ]` becomes `- [x]`, note spacing. You can also check the checkboxes after the PR is created. A detailed explanation of this checklist is available here: https://github.com/nvaccess/nvda/blob/master/devDocs/githubPullRequestTemplateExplanationAndExamples.md#code-review-checklist --> - [x] Pull Request description: - description is up to date - change log entries - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [ ] API is compatible with existing add-ons. - [ ] Documentation: - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [ ] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English You can view, comment on, or merge this pull request online at: #13226 -- Commit Summary -- * Scons: provide the number of logical processors as the number of maximum concurrent jobs to speed up builds -- File Changes -- M sconstruct (7) -- Patch Links -- https://github.com/nvaccess/nvda/pull/13226.patch https://github.com/nvaccess/nvda/pull/13226.diff -- Reply to this email directly or view it on GitHub: #13226 You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[seanbudd]

Is there a way to disable this for appveyor? Scrambling the output of try/PR builds and official builds without a time improvement seems like a loss.

[AppVeyorBot]

• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/1oa5exfpr364ickc/artifacts/output/

See test results for failed build of commit a3d4a2affe

[seanbudd]

Thanks @LeonarddeR

[michaelDCurran]

I'm really struggling with the merging of this pr. It is very hard to follow build output now, especially when trying to see errors from cl. This gets worse with the introduction of calling msbuild for the remote ops library.
Note that I have 12 logica processors, so there is a great deal going on at the same time.
I think parallel builds are a great advantage, but I do feel they should be optional. I.e. by specifying -j 12 (or how ever many you want) when running scons.
Pr #13367 has become necessary since this merge. And although we should probably take that one as parallel builds should work if people want them, I don't believe enforcing them by default, especially within a developer context is the right thing here.

I feel that this pr should be reverted.

[seanbudd]

@LeonarddeR @michaelDCurran can this be overridden with -j1 eg scons source -j1? This could either be a workaround, at least until the PR is reverted.

Would you like me to revert this? GitHub won't open a revert PR automatically

[lukaszgo1]

Have you reported the scrambled output thing to SCons?? While this would probably be pretty difficult to fix not reporting the problem makes it guaranteed not to be fixed.