Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: CMakeLists.txt forces full rebuild on every install #1769

Open
nicolasnoble opened this issue Oct 11, 2024 · 2 comments
Open

[Bug]: CMakeLists.txt forces full rebuild on every install #1769

nicolasnoble opened this issue Oct 11, 2024 · 2 comments

Comments

@nicolasnoble
Copy link

nicolasnoble commented Oct 11, 2024

Describe the issue

In its current state, the CMakeLists.txt of abseil inconditionally bypasses CMake's target longevity rules, and rewrites the file options.h every time CMake's consider the install rule during a build. This means this file gets touched on the filesystem inconditionally, forcing a rebuild of all of the descending targets which directly or indirectly depend on this file.

In practice, this means when a dependent is opting in to install abseil during its build, it effectively nullifies the caching mechanism of CMake, and forces a full rebuild of heavy hitter dependencies, such as gRPC, during CI testing for instance, but also when developers are iterating changes on their code.

Context:

abseil-cpp/CMakeLists.txt

Lines 216 to 255 in 4447c75

# Rewrite options.h to use the compiled ABI.
file(READ "absl/base/options.h" ABSL_INTERNAL_OPTIONS_H_CONTENTS)
# Handle features that require at least C++20.
if (ABSL_INTERNAL_AT_LEAST_CXX20)
foreach(FEATURE "ORDERING")
string(REPLACE
"#define ABSL_OPTION_USE_STD_${FEATURE} 2"
"#define ABSL_OPTION_USE_STD_${FEATURE} 1"
ABSL_INTERNAL_OPTIONS_H_PINNED
"${ABSL_INTERNAL_OPTIONS_H_CONTENTS}")
set(ABSL_INTERNAL_OPTIONS_H_CONTENTS "${ABSL_INTERNAL_OPTIONS_H_PINNED}")
endforeach()
endif()
# Handle features that require at least C++17.
if (ABSL_INTERNAL_AT_LEAST_CXX17)
foreach(FEATURE "ANY" "OPTIONAL" "STRING_VIEW" "VARIANT")
string(REPLACE
"#define ABSL_OPTION_USE_STD_${FEATURE} 2"
"#define ABSL_OPTION_USE_STD_${FEATURE} 1"
ABSL_INTERNAL_OPTIONS_H_PINNED
"${ABSL_INTERNAL_OPTIONS_H_CONTENTS}")
set(ABSL_INTERNAL_OPTIONS_H_CONTENTS "${ABSL_INTERNAL_OPTIONS_H_PINNED}")
endforeach()
endif()
# Any feature that still has the value of 2 (because it was not handled above)
# should be set to 0.
string(REGEX REPLACE
"#define ABSL_OPTION_USE_STD_([^ ]*) 2"
"#define ABSL_OPTION_USE_STD_\\1 0"
ABSL_INTERNAL_OPTIONS_H_PINNED
"${ABSL_INTERNAL_OPTIONS_H_CONTENTS}")
file(GENERATE OUTPUT "${CMAKE_BINARY_DIR}/options-pinned.h" CONTENT "${ABSL_INTERNAL_OPTIONS_H_PINNED}")
install(FILES "${CMAKE_BINARY_DIR}/options-pinned.h"
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/absl/base
RENAME "options.h")

Steps to reproduce the problem

This repository for instance is affected by this problem:

$ git clone https://github.com/triton-inference-server/client.git
$ cmake -S client -B triton-client-build -DTRITON_ENABLE_CC_GRPC=ON
$ make -C triton-client-build
$ make -C triton-client-build # observe this fully rebuilds gRPC and other targets

Use make -d -C triton-client-build to observe the following chosen extracts from the log:

[...]

-- Up-to-date: /home/nnoble/sources/triton/client/build/third-party/absl/include/absl/base/optimization.h
-- Installing: /home/nnoble/sources/triton/client/build/third-party/absl/include/absl/base/options.h
-- Up-to-date: /home/nnoble/sources/triton/client/build/third-party/absl/include/absl/base/policy_checks.h

[...]

     Prerequisite '/home/nnoble/sources/triton/client/build/third-party/absl/include/absl/base/options.h' is newer than target 'CMakeFiles/gpr.dir/src/core/lib/config/config_vars.cc.o'.
     Prerequisite '/home/nnoble/sources/triton/client/build/third-party/absl/include/absl/base/options.h' is newer than target 'CMakeFiles/gpr.dir/src/core/lib/config/config_vars_non_generated.cc.o'.
     Prerequisite '/home/nnoble/sources/triton/client/build/third-party/absl/include/absl/base/options.h' is newer than target 'CMakeFiles/gpr.dir/src/core/lib/config/load_config.cc.o'.

[...]

What version of Abseil are you using?

This happens at HEAD, and ever since this change has been introduced, on Sep 6, 2022, in 308bbf3

What operating system and version are you using?

Irrelevant for this problem, as it is CMake-related.

What compiler and version are you using?

Irrelevant for this problem, as it is CMake-related.

What build system are you using?

Any version of CMake is affected.

Additional context

As the change has been introduced, it has been slowly creeping up around the world's dependencies, and has since contributed to a significant CPU usage spike in CI jobs, and consumed an immense amount of developer-hours who are now waiting longer for their build while iterating on code. The effective dollar impact of the bug is difficult to properly estimate, but is definitely very large.

nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
@derekmauro
Copy link
Member

An internal contributor has a pending fix for this issue. Hopefully it will be submitted by Monday.

nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
nicolasnoble added a commit to nicolasnoble/triton-third_party that referenced this issue Oct 11, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
@nicolasnoble
Copy link
Author

I can confirm the following patch fixes the issue for me:

https://github.com/triton-inference-server/third_party/blob/a2a8ab22b1654b80ba3bad4594ffaf74990b1dd2/tools/abseil_no_rebuild.patch

nicolasnoble added a commit to triton-inference-server/third_party that referenced this issue Oct 14, 2024
Until abseil integrates this fix upstream, grpc picks up the new version of abseil with the fix, and until triton upgrades the grpc dependency, this will alleviate the repeated build issues in the meantime.
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

No branches or pull requests

2 participants