Execution platform selection for test actions in multi-platform builds

[fmeum]

The official discussion of the "Execution platform selection for test actions in multi-platform builds" proposal, which aims to select a reasonable execution platform for tests by default while remaining fully configurable for test rules with more complex requirements.

[katre]

This is really interesting (and absolutely intersects with my desire to figure out how to handle test platforms as they relate to both target and exec platforms).

I'd like to ask @ulfjack, @pcjanzen, and @keith to weigh in (as well as anyone else who is interested), to be sure this matches everyone's needs.

[keith]

I'm pretty confident this proposal covers our case. We are one of the customers in the folks who have been bugging @fmeum about this

[katre]

Hoisting this from a comment on the doc:

[R]esolve [a test's] execution platform to the first one that satisfies all constraints satisfied by the current target platform plus the constraints added by exec_compatible_with on rule or target

This is an interesting condition. I worry about custom targets platforms that add novel constraints that aren't present in any exec platform, however.

Would a better approach be:

1. Keep every exec platform that matches the target platform for constraint settings that they both declare (this will typically include both @platforms//os and @platforms//cpu)
2. Then pick the execution platform with the most matching constraints from step 1? (So an exec platform that only matches on cpu and os constraints is less preferred to one that matches on cpu, os, and a custom constraint)?

I also worry about the failure mode when no exec platform matches the target platform (using any definition of "matching"). Do we fall back to the first registered execution platform? Fail with an error?

[fmeum]

This is an interesting condition. I worry about custom targets platforms that add novel constraints that aren't present in any exec platform, however.

Do you have a real example in mind? I have definitely seen "creative" uses of target constraints that wouldn't make sense to transfer to the exec platform, but those really should have been exec constraints or just Starlark settings.

Generally speaking, if you have a binary built for the target platform, it may rely on any of the constraints on that platform to run successfully. I would be hesitant to make selection more magical.

If there really are valid reasons to have target constraints that make no sense on the exec platform, maybe we should just allow users to register their own default test toolchain? Such a toolchain could apply to all test exec groups that don't define their own toolchains.

I also worry about the failure mode when no exec platform matches the target platform (using any definition of "matching"). Do we fall back to the first registered execution platform? Fail with an error?

I would prefer failing with an error that tells the user how to proceed (register a new execution platform or turn certain constraints into target settings). If we had a way to register a default toolchain, that could be a good next step too.

[katre]

Do you have a real example in mind?

In the spirit of bazelbuild/platforms#100, I have seen cases where target platforms express additional constraints like "has AVX instructions" or "supports SSEv4", which the execution platforms do not. At that point it becomes random chance whether the actual execution platform used supports the same targeted micro-optimizations that the target platforms require.

I would be hesitant to make selection more magical.

I definitely agree with this.

I would prefer failing with an error that tells the user how to proceed

+1.

[fmeum]

In the spirit of bazelbuild/platforms#100, I have seen cases where target platforms express additional constraints like "has AVX instructions" or "supports SSEv4", which the execution platforms do not. At that point it becomes random chance whether the actual execution platform used supports the same targeted micro-optimizations that the target platforms require.

Based on my understanding of bazelbuild/platforms#100, the problem seems to be less that "has AVX instructions" is modeled as a constraint but rather that "x86_64 with AVX" should not be a value of the same constraint_setting as "x86_64 (generic)". If we instead had separate has_avx256, has_avx512, etc. constraint_settings with boolean values, I would consider it good practice to annotate your exec platforms with these constraints if you do the same with your target platforms. It's true that this may not be the case today, but it seems necessary if we want cross-platform execution to be reliable.

[fmeum]

I updated the proposal to make it more composable and less magic: Instead of augmenting resolution, I'm now advocating for adding a new generic test toolchain type that is the default for test exec groups as well as a default_test_toolchain rule that can be used to register a "target become exec constraints" toolchain even for custom test toolchain types.

[katre]

[The default_test_toolchain] behaves just like a regular toolchain target with these attributes plus both target_compatible_with and exec_compatible_with set to the list of constraints satisfied by the current target platform.

Where is this logic handled? Inside RegisteredToolchainsFunction (which does have access to the configuration), or inside the default_test_toolchain implementation? Ideally we'd be fetching the toolchain targets (from RegisteredToolchainsFunction) using the empty configuration to increase skyframe caching and reduce the total number of configured targets.

[fmeum]

I think that it can only be handled in one of the SkyFunctions since the target platform needs to be that of the target for which toolchains are resolved, not the target platform of the top-level configuration (or the empty configuration, with your proposed change).

[katre]

Please clarify this in the proposal, thanks.

[fmeum]

Done!

[katre]

add a mandatory requirement on a fixed new toolchain type @bazel_tools//tools/test:default_test_toolchain_type.

@comiusm @lberki : What's the current thinking about adding new toolchain types in @bazel_tools?

[fmeum]

@comius

This could also live in platforms.

[fmeum]

@comius @lberki Friendly ping

[katre]

Note that rules that do not want to be subject to this behavior, for example because they are expected to run on all platforms (e.g. they are based on Cosmopolitan Libc), can override this behavior by declaring a toolchain requirement and registering a toolchain with neither target nor exec constraints for it (or by explicitly setting toolchains = [] on the test exec group definition).

I'm not wild about using opt-out behavior here, would it make sense to instead make this opt-in (by having rules explicitly add toolchains = ["@bazel_tools//tools/test:default_test_toolchain_type"]?

@lberki , any thoughts?

[fmeum]

This would require almost all Starlark test rules to adopt an explicit test execution group to get a reasonable default and replace --use_target_platform_for_tests. I would consider this being the default behavior very important for this proposal to have the desired impact.

[fmeum]

@lberki Friendly ping

[lberki]

I haven't been following this thread so I'm afraid I can only offer generalities.

It looks like the motivation for this design is that --use_target_platform_for_tests is not good enough, but why is that in turn? The document lists a number of reasons, but they all boil down to (AFAICT) "the test platform needs to be different than the target platform", which I find a very strange concept because the whole reason to build a test is to execute it, so why make these two platforms different?

The design doc seems to want to add a new special toolchain type that influence the platform resolution for tests just the right way, but if that's the right approach, why not improve --use_target_platform_for_tests to fix the differences listed in this section of the design doc? That looks like a much more direct approach to the problem.

[fmeum]

I updated the doc section to clarify that --use_target_platform_for_tests only works in simple cases and by design (it forces the execution platform rather than going through resolution) doesn't work in combination with a number of Bazel features needed to make more complex remote builds work. I thus don't think that --use_target_platform_for_tests is "fixable".

The document lists a number of reasons, but they all boil down to (AFAICT) "the test platform needs to be different than the target platform", which I find a very strange concept because the whole reason to build a test is to execute it, so why make these two platforms different?

I added a reference to the cc_test and my_sh_test example that show that execution and target platform of a test might differ (think building for WASM, but testing in an interpreter). But if target and exec platform are semantically identical, then even with the proposed changes there would be no need to declare different platforms. It would only be necessary to ensure that the target platform is also registered as an exec platform, which is already the case in most builds.