Skip to content

Conversation

@thelvis4
Copy link

@thelvis4 thelvis4 commented Dec 19, 2025

After updating to apple_support 2.0.0, we noticed a significant (~40%) regression in compilation times.
For a large application (~45k clang compilations + few thousands swift module compilations), the overall build time increased from 38 min to 54 min.
The regression is rooted in changes in 2c64f60, where GetCanonicalPath is called for each argument, regardless if the replacement is needed (ie when code coverage is enabled) or not.
The overhead might vary, but it becomes significant and noticeable for compilation actions with lots of arguments.

This PR changes the logic for replacing __BAZEL_EXECUTION_ROOT_CANONICAL__ to avoid the overhead:

  1. coverage_prefix_map_absolute_sources_non_hermetic_private_feature sets COVERAGE_PREFIX_MAP_USE_ABSOLUTE_CWD_PATH=1 env
  2. GetCanonicalPath is called only if the environment variable is set (ie if the feature is requested) and its value is assigned to canonical_cwd
  3. canonical_cwd is passed down as a parameter and ProcessArgument performs the replacement if the parameter is non-empty

@keith
Copy link
Member

keith commented Dec 19, 2025

this change makes sense but instead of using a static variable here can you pass it down how cwd is passed down? or is the fact that it would still run once a problem there? if that is still a problem i think we should add an env var here

# A private feature that is used to embed absolute source paths in coverage builds.
#
# If enabled, coverage builds will use a `-coverage-prefix-map` that remaps
# the current working directory to a canonical location, which permits
# coverage representation in non-sandboxed builds with tools that expect
# absolute paths such as Xcode.
#
# This feature should only be used with non-sandboxed builds inside tools such as
# Xcode, and enabling it effectively breaks Bazel's ability to rely on the
# remote cache those builds. It should not be enabled by users of the toolchain.
coverage_prefix_map_absolute_sources_non_hermetic_private_feature = feature(
name = "_coverage_prefix_map_absolute_sources_non_hermetic",
flag_sets = [
flag_set(
actions = [
ACTION_NAMES.preprocess_assemble,
ACTION_NAMES.c_compile,
ACTION_NAMES.cpp_compile,
ACTION_NAMES.cpp_module_compile,
ACTION_NAMES.objc_compile,
ACTION_NAMES.objcpp_compile,
],
flag_groups = [
flag_group(
flags = ["-fcoverage-prefix-map=__BAZEL_EXECUTION_ROOT__=__BAZEL_EXECUTION_ROOT_CANONICAL__"],
),
],
),
],
requires = [feature_set(features = ["coverage"])],
)
and only calculate it when that is set

@thelvis4 thelvis4 force-pushed the reduce-GetCanonicalPath-overhead branch from 3a26df6 to 112b30d Compare December 22, 2025 23:03
@thelvis4
Copy link
Author

this change makes sense but instead of using a static variable here can you pass it down how cwd is passed down? or is the fact that it would still run once a problem there? if that is still a problem i think we should add an env var here

# A private feature that is used to embed absolute source paths in coverage builds.
#
# If enabled, coverage builds will use a `-coverage-prefix-map` that remaps
# the current working directory to a canonical location, which permits
# coverage representation in non-sandboxed builds with tools that expect
# absolute paths such as Xcode.
#
# This feature should only be used with non-sandboxed builds inside tools such as
# Xcode, and enabling it effectively breaks Bazel's ability to rely on the
# remote cache those builds. It should not be enabled by users of the toolchain.
coverage_prefix_map_absolute_sources_non_hermetic_private_feature = feature(
name = "_coverage_prefix_map_absolute_sources_non_hermetic",
flag_sets = [
flag_set(
actions = [
ACTION_NAMES.preprocess_assemble,
ACTION_NAMES.c_compile,
ACTION_NAMES.cpp_compile,
ACTION_NAMES.cpp_module_compile,
ACTION_NAMES.objc_compile,
ACTION_NAMES.objcpp_compile,
],
flag_groups = [
flag_group(
flags = ["-fcoverage-prefix-map=__BAZEL_EXECUTION_ROOT__=__BAZEL_EXECUTION_ROOT_CANONICAL__"],
),
],
),
],
requires = [feature_set(features = ["coverage"])],
)

and only calculate it when that is set

@keith updated the PR, please check if it's inline with what you had in mind

@thelvis4 thelvis4 changed the title Call GetCanonicalPath lazily and cache it to avoid FS overhead Call GetCanonicalPath only if needed to avoid FS overhead Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants