Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Improve CPU usage when building wasm_release #37294

Merged
merged 3 commits into from
Nov 4, 2022

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Nov 3, 2022

Doing a felt build (without --host) causes high cpu usage for some people (including me and @htoor3). This is because our build tries to use goma with a high parallelization factor that all ends up happening in the local machine because goma doesn't support emscripten yet.

This PR disables goma for wasm/web builds.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Nov 3, 2022
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Comment on lines 133 to 140
'autoninja',
// When build the wasm target that includes CanvasKit, we need to use
// `ninja` directly to avoid having a high `-j` value that could
// potentially slow down the entire machine because emscripten doesn't use
// goma.
host ? 'autoninja' : 'ninja',
Copy link
Member

Choose a reason for hiding this comment

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

'autoninja',
if (!host)
  '--offline',

may do the trick? That instructs autoninja to not look for goma, so it'll default to whatever J it deems good for local builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--offline did the trick!

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2022
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

Actually, maybe the right thing is to actually edit //flutter/tools/gn. It has a bunch of stuff to try to determine whether to use goma in there, and maybe we should just short circuit that if we're doing a wasm build.

@eyebrowsoffire eyebrowsoffire removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2022
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Works on my machine!

Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

LGTM!

@Hixie
Copy link
Contributor

Hixie commented Nov 4, 2022

test-exempt: configuration change

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 4, 2022
@auto-submit auto-submit bot merged commit 97fb982 into flutter:main Nov 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 4, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 4, 2022
…114667)

* 49acbfb08 Make iOS PlatformView to reuse VisualEffectView when possible. (flutter/engine#37263)

* 48f31a539 Roll Skia from 8e48bb8ea52e to 10acfb0efbc9 (2 revisions) (flutter/engine#37300)

* af61d4092 [Impeller] Add non-rrect polygon to shadow test (flutter/engine#37296)

* 97fb982f3 [web] Improve CPU usage when building wasm_release (flutter/engine#37294)

* ca0755adc Roll Skia from 10acfb0efbc9 to b8209dce9a48 (1 revision) (flutter/engine#37303)

* 44398ccf5 Make hot reload work (flutter/engine#37304)

* 69a275300 Roll Fuchsia Mac SDK from mOXbRSWGSdWRXIefR... to JKfnEvEVIL_Cg3_9f... (flutter/engine#37305)
schwa423 pushed a commit to schwa423/engine that referenced this pull request Nov 16, 2022
* [web] Improve CPU usage when building wasm_release

* use --offline mode

* make the change in tools/gn instead
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
…lutter#114667)

* 49acbfb08 Make iOS PlatformView to reuse VisualEffectView when possible. (flutter/engine#37263)

* 48f31a539 Roll Skia from 8e48bb8ea52e to 10acfb0efbc9 (2 revisions) (flutter/engine#37300)

* af61d4092 [Impeller] Add non-rrect polygon to shadow test (flutter/engine#37296)

* 97fb982f3 [web] Improve CPU usage when building wasm_release (flutter/engine#37294)

* ca0755adc Roll Skia from 10acfb0efbc9 to b8209dce9a48 (1 revision) (flutter/engine#37303)

* 44398ccf5 Make hot reload work (flutter/engine#37304)

* 69a275300 Roll Fuchsia Mac SDK from mOXbRSWGSdWRXIefR... to JKfnEvEVIL_Cg3_9f... (flutter/engine#37305)
@mdebbar mdebbar deleted the ninja_wasm branch January 17, 2023 18:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App needs tests platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants