ci: A few fixes of `ccache` issues #27084

hebasto · 2023-02-11T11:51:55Z

Combined #27077 and #27083 for the sake of more representative testing.

The second run looks promising:

"MSan" task -- 98.74 %
"ASan + LSan + UBSan + integer" task -- 100.0 %
"multiprocess" task -- 99.35 %
"macOS 13 native" task -- 100.0%
"Android APK" task -- 100.0%

Although, the "previous releases" task has a 66.10 % cache hit rate. It needs more investigation (not in this PR).

DrahtBot · 2023-02-11T11:51:58Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27360 (ci: use LLVM/clang-16 in native_asan job by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko · 2023-02-11T12:11:21Z

.cirrus.yml

@@ -5,8 +5,9 @@ env:  # Global defaults
  TEST_RUNNER_PORT_MIN: "14000"  # Must be larger than 12321, which is used for the http cache. See https://cirrus-ci.org/guide/writing-tasks/#http-cache
  CI_FAILFAST_TEST_LEAVE_DANGLING: "1"  # Cirrus CI does not care about dangling process and setting this variable avoids killing the CI script itself on error
  CCACHE_SIZE: "200M"
-  CCACHE_DIR: "/tmp/ccache_dir"
+  CCACHE_DIR: '${CIRRUS_WORKING_DIR}/ccache_dir'


This will still be in the same volume (tmp), so why is this changed?

I think the difference for Cirrus CI is not using absolute path /tmp/....

I am still not sure what this means and which paths are preferred or discouraged

#27084 (comment)

hebasto · 2023-02-11T15:29:55Z

The PR description has been updated with some statistics.

fanquake · 2023-02-20T17:02:34Z

Most of the changes here look straight forward enough, and clearly improve ccache usage, but this is blocked on at least an answer to this question: #27084 (comment).

hebasto · 2023-02-21T08:31:33Z

@fanquake @MarcoFalke

... but this is blocked on at least an answer to this question: #27084 (comment).

~~I've dropped the first commit and faced some weird issues which I saw in #27083 before. Going to provide steps to reproduce that unwanted behavior.~~

Making this PR a draft for now.

This change avoids any possible cache binary incompatibility.

By default, `ccache` includes the modification time (`mtime`) and size of the compiler in the hash to ensure that results retrieved from the cache are accurate. But in CI environment compiler's `mtime` can be unique each run.

This change improves cache hit rates for the touched CI tasks.

hebasto · 2023-02-21T12:59:20Z

The questionable commit has been dropped.

Ready to re-review.

maflcko

lgtm

maflcko · 2023-02-21T15:01:48Z

.cirrus.yml

@@ -36,6 +36,8 @@ main_template: &MAIN_TEMPLATE
  timeout_in: 120m  # https://cirrus-ci.org/faq/#instance-timed-out
  ccache_cache:
    folder: "/tmp/ccache_dir"
+    fingerprint_script: echo ${CIRRUS_TASK_NAME}


Isn't this already the default?

By default the task name is used as a fingerprint value.

https://cirrus-ci.org/guide/writing-tasks/#cache-instruction

Isn't this already the default?

Yep looks like it.

maflcko · 2023-02-21T15:02:44Z

.cirrus.yml

@@ -36,6 +36,8 @@ main_template: &MAIN_TEMPLATE
  timeout_in: 120m  # https://cirrus-ci.org/faq/#instance-timed-out
  ccache_cache:
    folder: "/tmp/ccache_dir"
+    fingerprint_script: echo ${CIRRUS_TASK_NAME}
+    reupload_on_changes: true


Isn't this already the default?

true otherwise

https://cirrus-ci.org/guide/writing-tasks/#cache-instruction

hebasto · 2023-03-01T09:16:13Z

Well, speaking of the removed questionable commit, I've finally managed to provide steps to reproduce unexpected behavior without that commit:

Complete all tasks.
Re-run, for example, the "32-bit + dash [gui] [CentOS 8]" task. As expected, ccache achieves 100% hit.
Add a commit which does not change any code and run CI job. It is natural to expect that ccache in the "32-bit + dash [gui] [CentOS 8]" task will 100% hit again. But that is not the case. Somehow, ccache's cache is "contaminated", which, in turn, causes some misses.

Besides the description above, I have no decent explanation of what is really happening. But s/tmp/ccache_dir/${CIRRUS_WORKING_DIR}/ccache_dir/ solves this issue.

maflcko · 2023-03-01T09:43:58Z

It would be good to double check if this is recommended "officially" by Cirrus CI. In any case CIRRUS_WORKING_DIR isn't under /tmp/ on macos, so maybe it makes sense for that reason already?

maflcko · 2023-03-01T09:44:54Z

I guess instead of ${CIRRUS_WORKING_DIR}/ccache_dir you can also just write ccache_dir, as CIRRUS_WORKING_DIR is implicit?

hebasto · 2023-03-01T11:51:05Z

I guess instead of ${CIRRUS_WORKING_DIR}/ccache_dir you can also just write ccache_dir, as CIRRUS_WORKING_DIR is implicit?

While it works for the folder property of the ccache_cache, it seems the CCACHE_DIR variable is supposed to contain an absolute path.

maflcko · 2023-03-13T16:21:43Z

lgtm. While it would be good to be able to point to docs, I don't think this is required. Happy to review if you move this out of draft.

DrahtBot · 2023-05-02T13:44:42Z

🐙 This pull request conflicts with the target branch and needs rebase.

fanquake · 2023-05-16T12:05:57Z

What is the status of this

hebasto · 2023-05-17T14:42:28Z

What is the status of this

I've analyzed the recent ccache summaries at a75c77e.

I don't think this PR can improve them.

DrahtBot added the Tests label Feb 11, 2023

maflcko reviewed Feb 11, 2023

View reviewed changes

hebasto marked this pull request as ready for review February 11, 2023 15:20

hebasto marked this pull request as draft February 21, 2023 08:31

hebasto added 3 commits February 21, 2023 12:56

ci: Distinguish ccache cache by tasks

0df55c5

This change avoids any possible cache binary incompatibility.

ci: Make ccache hashing content of compiler binary

3b76252

By default, `ccache` includes the modification time (`mtime`) and size of the compiler in the hash to ensure that results retrieved from the cache are accurate. But in CI environment compiler's `mtime` can be unique each run.

ci: Adjust CCACHE_SIZE for some tasks

9a5c00c

This change improves cache hit rates for the touched CI tasks.

hebasto marked this pull request as ready for review February 21, 2023 12:58

hebasto force-pushed the 230211-cccc branch from b49fdb1 to 9a5c00c Compare February 21, 2023 12:58

maflcko approved these changes Feb 21, 2023

View reviewed changes

hebasto marked this pull request as draft February 27, 2023 14:53

This was referenced Mar 29, 2023

ci: use LLVM/clang-16 in native_asan job #27360

Merged

build: Add CMake-based build system #25797

Closed

DrahtBot mentioned this pull request Apr 21, 2023

ci: drop untrusted bpfcc tool usage #27510

Closed

DrahtBot added the Needs rebase label May 2, 2023

hebasto closed this May 17, 2023

bitcoin locked and limited conversation to collaborators May 16, 2024

ci: A few fixes of ccache issues #27084

ci: A few fixes of ccache issues #27084

Uh oh!

Conversation

hebasto commented Feb 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Feb 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

maflcko Feb 11, 2023

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Feb 13, 2023

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

hebasto commented Feb 11, 2023

Uh oh!

fanquake commented Feb 20, 2023

Uh oh!

hebasto commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Feb 21, 2023

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

fanquake Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

maflcko Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

hebasto commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Mar 1, 2023

Uh oh!

maflcko commented Mar 1, 2023

Uh oh!

hebasto commented Mar 1, 2023

Uh oh!

maflcko commented Mar 13, 2023

Uh oh!

DrahtBot commented May 2, 2023

Uh oh!

fanquake commented May 16, 2023

Uh oh!

hebasto commented May 17, 2023

Uh oh!

Uh oh!

ci: A few fixes of `ccache` issues #27084

ci: A few fixes of `ccache` issues #27084

hebasto commented Feb 11, 2023 •

edited

Loading

DrahtBot commented Feb 11, 2023 •

edited

Loading

hebasto Feb 11, 2023 •

edited

Loading

hebasto commented Feb 21, 2023 •

edited

Loading

hebasto commented Mar 1, 2023 •

edited

Loading