Compile Apple tools as fat binaries if possible #13452
Conversation
|
Note this doesn't touch xcode-locator because that's not an input to other actions (or the embedded version is used instead) |
keith
force-pushed
the
ks/compile-apple-tools-as-fat-binaries-if-possible
branch
from
519c82f to
53100e4
Compare
| "-arch", | ||
| "arm64", | ||
| "-arch", | ||
| "x86_64", |
There was a problem hiding this comment.
This is the only difference from the function above, but since you can't recursively call functions in starlark I had to duplicate this
|
I tested this with Xcode 11.7 and it works as expected by falling back to the single arch binary |
keith
force-pushed
the
ks/compile-apple-tools-as-fat-binaries-if-possible
branch
from
53100e4 to
81c98ca
Compare
|
AFAIK, this is needed in order to kick off macOS remote execution from an Apple Silicon machine where the executors are Intel. |
|
@kaylathar Per this #13873 (comment), could you please take a look at this? 🙏 |
|
@keith Are you using this internally? We noticed significant cache misses after using this patch. |
|
We're not yet, we're only testing M1s atm. Did you debug the misses back to this? |
|
I did a quick debug and it turned out that the fat wrapped_clang is not deterministic. |
|
I can confirm that our cache hit rate went back to normal after reverting this change.
Update: Building for each arch and manually lipo'ing yields a consistent binary. |
keith
force-pushed
the
ks/compile-apple-tools-as-fat-binaries-if-possible
branch
from
81c98ca to
29eabbe
Compare
The Apple toolchain has 2 native binaries that are inputs to every single action. Because of this if you want to share caches between Apple Silicon machines and Intel machines, you either need to force them to be x86_64 binaries and suffer the performance loss on Apple Silicon machiens, or use fat binaries so the sha's match on both architectures, which is what this change does. These binaries are so small that the size impact of this doesn't matter. Since Apple Silicon support requires Xcode 12 this falls back to compiling the single architecture binary if it fails, under the assumption that means you're on Xcode 11 or lower. We don't have a better indication at this point of what Xcode version you're using, so this seems like a fine workaround until Xcode 12 is the minimum supported version.
keith
force-pushed
the
ks/compile-apple-tools-as-fat-binaries-if-possible
branch
from
29eabbe to
b075144
Compare
|
@kaylathar this would be great to get in before the 5.0 cut 🙏 |
|
@oquenchil can you import? |
|
LGTM for merge |
|
@comius Can you import this? 🙏 |
|
Imported. |
|
@comius thanks, I don't see it on master yet, is something stuck? |
The Apple toolchain has 2 native binaries that are inputs to every single action. Because of this if you want to share caches between Apple Silicon machines and Intel machines, you either need to force them to be x86_64 binaries and suffer the performance loss on Apple Silicon machiens, or use fat binaries so the sha's match on both architectures, which is what this change does. These binaries are so small that the size impact of this doesn't matter. Since Apple Silicon support requires Xcode 12 this falls back to compiling the single architecture binary if it fails, under the assumption that means you're on Xcode 11 or lower. We don't have a better indication at this point of what Xcode version you're using, so this seems like a fine workaround until Xcode 12 is the minimum supported version. Closes bazelbuild#13452. PiperOrigin-RevId: 405842940 (cherry picked from commit 80c56ff)
The Apple toolchain has 2 native binaries that are inputs to every single action. Because of this if you want to share caches between Apple Silicon machines and Intel machines, you either need to force them to be x86_64 binaries and suffer the performance loss on Apple Silicon machiens, or use fat binaries so the sha's match on both architectures, which is what this change does. These binaries are so small that the size impact of this doesn't matter. Since Apple Silicon support requires Xcode 12 this falls back to compiling the single architecture binary if it fails, under the assumption that means you're on Xcode 11 or lower. We don't have a better indication at this point of what Xcode version you're using, so this seems like a fine workaround until Xcode 12 is the minimum supported version. Closes #13452. PiperOrigin-RevId: 405842940 (cherry picked from commit 80c56ff)
The Apple toolchain has 2 native binaries that are inputs to every
single action. Because of this if you want to share caches between Apple
Silicon machines and Intel machines, you either need to force them to be
x86_64 binaries and suffer the performance loss on Apple Silicon
machiens, or use fat binaries so the sha's match on both architectures,
which is what this change does. These binaries are so small that the
size impact of this doesn't matter. Since Apple Silicon support requires
Xcode 12 this falls back to compiling the single architecture binary if
it fails, under the assumption that means you're on Xcode 11 or lower.
We don't have a better indication at this point of what Xcode version
you're using, so this seems like a fine workaround until Xcode 12 is the
minimum supported version.