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

Run clang tidy builds on arm Macs #41183

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Apr 14, 2023

Intel Mac capacity is limited. Swap the two Mac clang-tidy builders to arm bots. The clang-tidy linter should run with arm natively.

Mac Host clang-tidy

Prod x64: https://ci.chromium.org/p/flutter/builders/prod/Mac%20Host%20clang-tidy/3318 Execution 35 mins 2 secs
This PR arm: https://ci.chromium.org/p/flutter/builders/try/Mac%20Host%20clang-tidy/8087 Execution 36 mins 30 secs (this hit an Xcode cache miss which added 4 minutes)

Mac iOS clang-tidy

Prod x64: https://ci.chromium.org/p/flutter/builders/prod/Mac%20iOS%20clang-tidy/3303 Execution 11 mins 57 secs
This PR arm: https://ci.chromium.org/p/flutter/builders/try/Mac%20iOS%20clang-tidy/8087 Execution 18 mins 10 secs (this hit an Xcode cache miss which added 5 minutes)

@jmagman jmagman self-assigned this Apr 14, 2023
@@ -383,7 +383,7 @@ targets:
recipe: engine/engine_lint
properties:
add_recipes_cq: "true"
cores: "12"
Copy link
Member Author

@jmagman jmagman Apr 14, 2023

Choose a reason for hiding this comment

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

The arm bots have 8 cores but should still be pretty fast.

@@ -383,7 +383,8 @@ targets:
recipe: engine/engine_lint
properties:
add_recipes_cq: "true"
cores: "12"
cores: "8"
Copy link
Member Author

Choose a reason for hiding this comment

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

This dimension should be removed, but when I do the try falls back to 12 in the upstreamed config and fails with no resource https://chromium-swarm.appspot.com/task?id=619612e614a28f10&o=true&w=true.

The ones in prod are also 8 core so this should still run there. I'll remove it when the config syncs after merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, all arms are with 8 cores. Why not removing the cores from this PR?

Copy link
Member Author

@jmagman jmagman Apr 14, 2023

Choose a reason for hiding this comment

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

Yeah, all arms are with 8 cores. Why not removing the cores from this PR?

When I remove it the build fails with resource unavailable: https://chromium-swarm.appspot.com/task?id=619612e614a28f10&o=true&w=true.

I assumed it's not passing in "no cores", it's falling back to the synced config that says 12? I was thinking I would merge this and then try to remove the cores part in a future PR.

Copy link
Member Author

@jmagman jmagman Apr 14, 2023

Choose a reason for hiding this comment

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

I'm going to merge this and then remove the cores dimension in the next PR. I'll add you as a reviewer.

@jmagman jmagman requested review from zanderso and keyonghan April 14, 2023 01:26
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #41183 at sha 7677a55

@jmagman jmagman added autosubmit Merge PR when tree becomes green via auto submit App and removed will affect goldens labels Apr 14, 2023
@auto-submit auto-submit bot merged commit 69645ed into flutter:main Apr 14, 2023
@jmagman jmagman deleted the clang-tidy-arm branch April 14, 2023 18:52
auto-submit bot pushed a commit that referenced this pull request Apr 14, 2023
Follow up to #41183.  Remove the `cores` dimensions so this builder can run on any arm machine which currently all have 8 cores but there's no reason to specify now that the arch is arm.

Introduced in #38261 to avoid 4-core Intel machines.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 14, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 14, 2023
…124896)

flutter/engine@413e95d...e9739bc

2023-04-14 [email protected] [macOS] Build unittests on all macOS host builds (flutter/engine#41215)
2023-04-14 [email protected] [web] Don't run goldctl init more than once (flutter/engine#41207)
2023-04-14 [email protected] Roll Skia from 2bd12e31d578 to 22e417bea884 (4 revisions) (flutter/engine#41213)
2023-04-14 [email protected] Revert "[Android] Send connectionClosed message when keyboard becomes invisible to ensure framework focus state is correct." (flutter/engine#41211)
2023-04-14 [email protected] Remove `Mac mac_android_aot_engine` in favor of Linux (flutter/engine#41210)
2023-04-14 [email protected] Run clang tidy builds on arm Macs (flutter/engine#41183)
2023-04-14 [email protected] Roll Fuchsia Linux SDK from diD1gLr_dKWFJlsSn... to Z0of2S9pf3Zn1nsJP... (flutter/engine#41209)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from diD1gLr_dKWF to Z0of2S9pf3Zn

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
…lutter#124896)

flutter/engine@413e95d...e9739bc

2023-04-14 [email protected] [macOS] Build unittests on all macOS host builds (flutter/engine#41215)
2023-04-14 [email protected] [web] Don't run goldctl init more than once (flutter/engine#41207)
2023-04-14 [email protected] Roll Skia from 2bd12e31d578 to 22e417bea884 (4 revisions) (flutter/engine#41213)
2023-04-14 [email protected] Revert "[Android] Send connectionClosed message when keyboard becomes invisible to ensure framework focus state is correct." (flutter/engine#41211)
2023-04-14 [email protected] Remove `Mac mac_android_aot_engine` in favor of Linux (flutter/engine#41210)
2023-04-14 [email protected] Run clang tidy builds on arm Macs (flutter/engine#41183)
2023-04-14 [email protected] Roll Fuchsia Linux SDK from diD1gLr_dKWFJlsSn... to Z0of2S9pf3Zn1nsJP... (flutter/engine#41209)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from diD1gLr_dKWF to Z0of2S9pf3Zn

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants