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

[CI][C++] Bad compile/link times despite ccache #35641

Closed
pitrou opened this issue May 17, 2023 · 8 comments · Fixed by #37502
Closed

[CI][C++] Bad compile/link times despite ccache #35641

pitrou opened this issue May 17, 2023 · 8 comments · Fixed by #37502

Comments

@pitrou
Copy link
Member

pitrou commented May 17, 2023

Describe the bug, including details regarding any error messages, version, and platform.

Example here on the AMD64 Conda C++ build:
https://github.com/apache/arrow/actions/runs/5005430904/jobs/8969381538?pr=33634

=== ccache statistics before build
===
Cacheable calls:   284 / 894 (31.77%)
  Hits:             14 / 284 ( 4.93%)
    Direct:         14 /  14 (100.0%)
    Preprocessed:    0 /  14 ( 0.00%)
  Misses:          270 / 284 (95.07%)
Uncacheable calls: 610 / 894 (68.23%)
Local storage:
  Cache size (GB): 0.1 / 1.0 (11.35%)
  Hits:             14 / 284 ( 4.93%)
  Misses:          270 / 284 (95.07%)

[ ... CMake runs ... ]

real	27m46.563s
user	49m43.936s
sys	4m28.705s

===
=== ccache statistics after build
===
Cacheable calls:    568 / 1788 (31.77%)
  Hits:             298 /  568 (52.46%)
    Direct:         233 /  298 (78.19%)
    Preprocessed:    65 /  298 (21.81%)
  Misses:           270 /  568 (47.54%)
Uncacheable calls: 1220 / 1788 (68.23%)
Local storage:
  Cache size (GB):  0.1 /  1.0 (11.35%)
  Hits:             298 /  568 (52.46%)
  Misses:           270 /  568 (47.54%)

So ccache was completely successful in this build (0 misses during the entire CMake configure / build), yet CMake took 27 minutes.

We should investigate:

  • disabling building static libraries (ARROW_BUILD_STATIC) to save on linking times
  • why there are more than 600 uncacheable calls and only 280 cacheable calls, which is the majority of the ~860 compilation calls (the "Building CXX" lines in the logs)

Note that the "AMD64 Conda C++" build can be reproduced using Archery with the conda-cpp image:

$ archery docker run conda-cpp

Component(s)

C++

@pitrou
Copy link
Member Author

pitrou commented May 17, 2023

cc @kou @felipecrv @assignUser @bkietz

@pitrou
Copy link
Member Author

pitrou commented May 17, 2023

It seems "Could not use precompiled header" is the reason, at least on conda-cpp, for uncacheable compiler calls.

See also: GH-35642

@kou
Copy link
Member

kou commented May 17, 2023

If precompiled header is a problem, we may be able to fix this by the following change:

diff --git a/docker-compose.yml b/docker-compose.yml
index 588146075..96b4290f5 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -281,7 +281,6 @@ services:
       ARROW_EXTRA_ERROR_CONTEXT: "ON"
       ARROW_MIMALLOC: "ON"
       ARROW_USE_LD_GOLD: "ON"
-      ARROW_USE_PRECOMPILED_HEADERS: "ON"
       BUILD_DOCS_PYTHON: "ON"
     volumes: &conda-volumes
       - .:/arrow:delegated

@assignUser
Copy link
Member

hm interesting investigation, thanks @pitrou !

@pitrou
Copy link
Member Author

pitrou commented May 18, 2023

If precompiled header is a problem, we may be able to fix this by the following change:

Yes, I think we should disable precompiled headers, and perhaps enable unity builds instead.

@pitrou
Copy link
Member Author

pitrou commented May 18, 2023

@felipecrv Do you want to take a look at this?

@felipecrv felipecrv self-assigned this May 18, 2023
@felipecrv
Copy link
Contributor

I'm unassigning myself for now because I have too many open workstreams that I want to close right now.

@felipecrv felipecrv removed their assignment May 23, 2023
@pitrou
Copy link
Member Author

pitrou commented May 23, 2023

No problem, @felipecrv ! @benibus, are you interested in taking a look at this?

@benibus benibus self-assigned this May 23, 2023
pitrou added a commit that referenced this issue Sep 1, 2023
### Rationale for this change

ccache does not like precompiled headers and does not cache anything in that case, which pessimizes build times.

### What changes are included in this PR?

Disable precompiled headers, and enable unity builds to make up for it.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.

* Closes: #35641

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 14.0.0 milestone Sep 1, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
### Rationale for this change

ccache does not like precompiled headers and does not cache anything in that case, which pessimizes build times.

### What changes are included in this PR?

Disable precompiled headers, and enable unity builds to make up for it.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.

* Closes: apache#35641

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
### Rationale for this change

ccache does not like precompiled headers and does not cache anything in that case, which pessimizes build times.

### What changes are included in this PR?

Disable precompiled headers, and enable unity builds to make up for it.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.

* Closes: apache#35641

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants