This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Split up mac_host_engine builds #53571
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zanderso
force-pushed
the
try-split-macos-build
branch
from
b3096bd to
57cb1bc
Compare
zanderso
force-pushed
the
try-split-macos-build
branch
10 times, most recently
from
7bee222 to
bb37281
Compare
zanderso
added a commit
to flutter/flutter
that referenced
this pull request
Jul 12, 2024
zanderso
force-pushed
the
try-split-macos-build
branch
3 times, most recently
from
0cee18f to
45cedee
Compare
zanderso
added a commit
to flutter/flutter
that referenced
this pull request
Jul 25, 2024
zanderso
force-pushed
the
try-split-macos-build
branch
from
45cedee to
ffdcf29
Compare
cbracken
approved these changes
Jul 26, 2024
Member
cbracken
left a comment
cbracken
left a comment
There was a problem hiding this comment.
Nice. The presubmit in the try patch appears to have a couple failures but they look entirely unrelated to this.
jmagman
reviewed
Jul 26, 2024
Member
There was a problem hiding this comment.
My understanding of the original intention of the coordinator builds was that the "builds" part would build only the dependencies needed for a test (or uploaded tool artifact), then the "tests" part would run the related tests and pull down the dependencies. Like:
display builds
- ci/host_debug
display tests
- ci/host_debug_tests
But this is instead gathering all of the builds together and the coordinator is waiting for them, then another coordinator is gathering all the tests (but as "builds") and then waiting for them. The tests Mac mac_unopt (tests) could run before Mac mac_host_engine (artifacts) and I'm assuming it's not doing duplicate work because of RBE?
If we are relying on RBE to avoid duplicate building, why do we have coordinators? Can we get rid of them since they seem to be hogging up a builder, and are confusing when you look at the dashboard to figure out what's going on?
ci/builders/mac_host_engine.json
Outdated
Comment on lines
416
to
421
| "dependencies": [ | ||
| { | ||
| "dependency": "goldctl", | ||
| "version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd" | ||
| } | ||
| ], |
Member
There was a problem hiding this comment.
I don't think this one tests goldens, but I may be missing it:
https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20Engine%20Drone/1019975/overview
zanderso
added a commit
to flutter/cocoon
that referenced
this pull request
Jul 26, 2024
Member
Author
|
Jenn and I chatted offline. This PR has to wait for this PR flutter/cocoon#3832 to be deployed before it can land. |
jmagman
approved these changes
Jul 26, 2024
Member
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
Can we get rid of them since they seem to be hogging up a builder
Actually the mac_host_engine runs generators after, so that one does something.
Talked to @zanderso, looks like it would require some recipe voodoo to remove the coordinator for the unopt build. LGTM, this is definitely an improvement.
zanderso
force-pushed
the
try-split-macos-build
branch
from
ffdcf29 to
629f78b
Compare
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 27, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jul 27, 2024
…152417) flutter/engine@0c5504e...739ae15 2024-07-26 [email protected] Split up mac_host_engine builds (flutter/engine#53571) 2024-07-26 [email protected] [Impeller] Fix the operator used to build the condition to check for OpenGLES. (flutter/engine#54155) 2024-07-26 [email protected] Roll Skia from 4a97d01dfedd to 9632ef8f18c1 (7 revisions) (flutter/engine#54154) 2024-07-26 [email protected] Roll Dart SDK from de1925e18998 to 65bb9e8275bc (1 revision) (flutter/engine#54153) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TytaniumDev
pushed a commit
to TytaniumDev/flutter
that referenced
this pull request
Aug 7, 2024
…lutter#152417) flutter/engine@0c5504e...739ae15 2024-07-26 [email protected] Split up mac_host_engine builds (flutter/engine#53571) 2024-07-26 [email protected] [Impeller] Fix the operator used to build the condition to check for OpenGLES. (flutter/engine#54155) 2024-07-26 [email protected] Roll Skia from 4a97d01dfedd to 9632ef8f18c1 (7 revisions) (flutter/engine#54154) 2024-07-26 [email protected] Roll Dart SDK from de1925e18998 to 65bb9e8275bc (1 revision) (flutter/engine#54153) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Buchimi
pushed a commit
to Buchimi/flutter
that referenced
this pull request
Sep 2, 2024
…lutter#152417) flutter/engine@0c5504e...739ae15 2024-07-26 [email protected] Split up mac_host_engine builds (flutter/engine#53571) 2024-07-26 [email protected] [Impeller] Fix the operator used to build the condition to check for OpenGLES. (flutter/engine#54155) 2024-07-26 [email protected] Roll Skia from 4a97d01dfedd to 9632ef8f18c1 (7 revisions) (flutter/engine#54154) 2024-07-26 [email protected] Roll Dart SDK from de1925e18998 to 65bb9e8275bc (1 revision) (flutter/engine#54153) 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://issues.skia.org/issues/new?component=1389291&template=1850622 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.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR does a few things. Mainly it makes the builds in mac_host_engine.json each build fewer targets to increase parallelism for post-submit and release builds. To ensure that increasing parallelism doesn't lead to capacity issues, this change also allows the mac_host_engine.json builds to run on either intel or arm64 macOS hosts. Finally, when building on an arm64 macOS host to target macOS, this PR changes the
gnscript to ensure that the arm64 native clang toolchain will be used.To keep mac_host_engine.json focused on builds that produce artifacts, this PR also moves tests from that file into the ill-named mac_unopt.json. In a subsequent PR, I'll rename all the *_unopt.json files to *_tests.json or something similar.
The artifacts produced by these builds are passing framework presubmit checks in flutter/flutter#152345.