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

Base.stale_cachefile: allow ftime_req to be greater than ftime by up to one microsecond (to compensate for Windows tar) #45552

Merged
merged 1 commit into from
Jun 17, 2022

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Jun 1, 2022

This PR modifies the Base.stale_cachefile function, which is used in code loading to determine whether or not a .ji cachefile is fresh relative to its .jl source files.

In this PR, for each source file, we allow ftime_req (the original mtime of the source file as recorded in the header of the cachefile) to be greater than ftime (the current mtime of the source file), but only if ftime_req - ftime is strictly greater than zero and strictly less than one microsecond. In other words, we allow a cachefile to be considered fresh if the following condition is true: 0 < (ftime_req - ftime) < 1e-6.

Motivation

I am running into the following issue:

  1. On GitHub Actions CI, trigger a new CI job on a GitHub-hosted Windows runner. This job runs inside a brand new virtual machine.
  2. During the job, create a new depot, activate a new environment, add some packages, and precompile the environment.
  3. At the end of the job, the "upload" step of the actions/cache action runs, which performs the following steps: tar up the depot, upload the tarball to GitHub's servers.
  4. Trigger another CI job, again on a GitHub-hosted Windows runner. This job runs inside a brand new virtual machine that has identical specs to the previous virtual machine.
  5. At the beginning of the job, the "download" step of the actions/cache action runs, which performs the following steps: download the tarball from GitHub's servers, extract the tarball to the exact same location as before.
  6. Set ENV["JULIA_DEBUG"] = "all" and do import SomePackage.

Expected behavior: all of the cachefiles should be fresh.

Observed behavior: at least one of the cachefiles is stale. The @debug message shows that ftime != ftime_req. More specifically, ftime_req > ftime. However, in all of the tests that I have done, it has always been the case that 0 < (ftime_req - ftime) < 1e-6.

This makes it impossible to use the GitHub Actions CI cache to cache the Julia precompilation cache for Windows jobs.

Interestingly enough, I only see this issue on Windows. Everything works fine on the GitHub-hosted Linux runners.

@DilumAluthge DilumAluthge changed the title Base.stale_cachefile: allow ftime to be greater than ftime_req by up to one microsecond (to compensate for Windows tar) Base.stale_cachefile: allow ftime_req to be greater than ftime by up to one microsecond (to compensate for Windows tar) Jun 1, 2022
@DilumAluthge DilumAluthge marked this pull request as ready for review June 2, 2022 03:36
@DilumAluthge DilumAluthge force-pushed the dpa/stale_cachefile-mtime-windows-microsecond branch from 28db101 to ed3876e Compare June 2, 2022 03:37
@DilumAluthge DilumAluthge added packages Package management and loading compiler:precompilation Precompilation of modules labels Jun 2, 2022
Copy link
Sponsor Member

@staticfloat staticfloat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix makes sense, but I don't understand the root cause; why is windows' tar fudging our timestamps?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 3, 2022

More specially, how does it fudge it this oddly. Is it specifically rounding up (since we already handle rounding down)?

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 4, 2022

Yeah, I would really like to understand why this is happening.

As far as I can tell, all of the @debug messages look like this:

┌ Debug: Rejecting stale cache file C:\path\to\depot\.julia\compiled\v1.7\CSV\HHBkp_0PArC.ji (mtime 1.654090805352805e9) because file C:\path\to\depot\.julia\packages\CSV\0Elut\src\write.jl (mtime 1.654090805352804e9) has changed

It's always just that last digit that differs.

…by up to one microsecond (to compensate for Windows tar)
@DilumAluthge DilumAluthge force-pushed the dpa/stale_cachefile-mtime-windows-microsecond branch from ed3876e to be650ac Compare June 16, 2022 10:13
@DilumAluthge DilumAluthge merged commit 4c39647 into master Jun 17, 2022
@DilumAluthge DilumAluthge deleted the dpa/stale_cachefile-mtime-windows-microsecond branch June 17, 2022 08:41
@DilumAluthge DilumAluthge added backport 1.6 Change should be backported to release-1.6 backport 1.7 backport 1.8 Change should be backported to release-1.8 labels Oct 10, 2022
KristofferC pushed a commit that referenced this pull request Oct 27, 2022
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
@KristofferC KristofferC mentioned this pull request Oct 27, 2022
37 tasks
KristofferC pushed a commit that referenced this pull request Oct 28, 2022
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Nov 8, 2022
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
KristofferC pushed a commit that referenced this pull request Oct 10, 2023
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.6 Change should be backported to release-1.6 compiler:precompilation Precompilation of modules packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants