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

Parallel precompile now seems to fail precompiling the package itself #53859

Closed
KristofferC opened this issue Mar 26, 2024 · 9 comments · Fixed by #53905
Closed

Parallel precompile now seems to fail precompiling the package itself #53859

KristofferC opened this issue Mar 26, 2024 · 9 comments · Fixed by #53905
Assignees
Labels
compiler:precompilation Precompilation of modules regression Regression in behavior compared to a previous version
Milestone

Comments

@KristofferC
Copy link
Member

KristofferC commented Mar 26, 2024

(Foo) pkg> precompile
   Installed Example ─ v0.5.3
Precompiling all packages...
  1 dependency successfully precompiled in 1 seconds

(Foo) pkg> st
Status `~/julia/Foo/Project.toml`
  [7876af07] Example v0.5.3

julia> using Foo # should have precompiled `Foo` already
[ Info: Precompiling Foo [16a8b8d3-1370-5883-bb6f-f9266484db3f] 
@KristofferC KristofferC added regression Regression in behavior compared to a previous version compiler:precompilation Precompilation of modules labels Mar 26, 2024
@KristofferC KristofferC added this to the 1.12 milestone Mar 26, 2024
@KristofferC KristofferC self-assigned this Mar 26, 2024
@KristofferC KristofferC modified the milestones: 1.12, 1.11 Mar 26, 2024
@KristofferC
Copy link
Member Author

(Might actually not be an issue on 1.11)

@krynju
Copy link
Contributor

krynju commented Mar 29, 2024

I hit a similiar case with one package on both 1.11 and nightly (28.03.24)
I would also get precompile invalidations and runtime precompile on packages depending on this package during build

➜  Abc git:(main) ✗ julia +1.11
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.11.0-alpha2 (2024-03-18)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(@v1.11) pkg> activate .
  Activating project at `~/P/packages/Abc`

julia> using Abc
Precompiling Abc...
  1 dependency successfully precompiled in 5 seconds. 128 already precompiled.
[ Info: Precompiling Abc [6198f965-9602-40a9-911d-ddd886f47855] (cache misses: nonresolveable depot (2))

julia> using Abc

(Abc) pkg> instantiate
Precompiling project...
  ✓ Abc
  1 dependency successfully precompiled in 5 seconds. 129 already precompiled.
  1 dependency precompiled but a different version is currently loaded. Restart julia to access the new version

(Abc) pkg> instantiate
Precompiling project...
  ✓ Abc
  1 dependency successfully precompiled in 4 seconds. 129 already precompiled.
  1 dependency precompiled but a different version is currently loaded. Restart julia to access the new version



➜  Abc git:(main) ✗ julia +nightly
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.12.0-DEV.265 (2024-03-28)
 _/ |\__'_|_|_|\__'_|  |  Commit b18d2cc7046 (0 days old master)
|__/                   |

(@v1.12) pkg> activate .
  Activating project at `~/P/packages/Abc`

julia> using Abc
Precompiling Abc...
  1 dependency successfully precompiled in 5 seconds. 128 already precompiled.
[ Info: Precompiling Abc [6198f965-9602-40a9-911d-ddd886f47855] (cache misses: nonresolveable depot (2))

(Abc) pkg> instantiate
Precompiling all packages...
  ✓ Abc
  1 dependency successfully precompiled in 5 seconds. 128 already precompiled.
  1 dependency precompiled but a different version is currently loaded. Restart julia to access the new version

@fatteneder
Copy link
Member

Do you have a minimal reproducer?
Or is the source code of the offending pkgs openly available?

@krynju
Copy link
Contributor

krynju commented Mar 29, 2024

It's proprietary code, but I managed to get an MWE

I think this issue is different from what OP reported, but maybe it's related? Looks more similiar to #53831
I couldn't reproduce the issue from OP on other packages

module Abc

using AWS: @service

@service ECR

end

AWS.jl as the only dependecy

Found three packages like this and this was the common thing in them

@fatteneder
Copy link
Member

fatteneder commented Mar 29, 2024

In #52346 we already addressed an issue related to the AWS pkg. @IanButterworth

Will need to have a closer look again.

@fatteneder
Copy link
Member

@KristofferC What's the setup to reproduce the Example.jl issue?
I can't reproduce on 313f933 or #53905.

@IanButterworth
Copy link
Member

The active project needs to be a package. That should be all that's required I believe.

@fatteneder
Copy link
Member

I cloned Example.jl and tried with

  • activating its package environment -- no recompilation
  • generating a separate project and adding Example.jl as a dependency -- no recompilation

In any case, if the original issue was related to relocation, then I think it should be fixed after #53905.

fatteneder added a commit that referenced this issue Apr 2, 2024
…files (#53905)

Fixes
#53859 (comment),
which was actually fixed before in
#52346, but
#52750 undid that fix.

This PR does not directly address #53859, because I can not reproduce it
atm.

---

The `@depot` resolution logic for `include()` files is adjusted as
follows:

1. (new behavior) If the cache is not relocatable because of an absolute
path, we ignore that path for the depot search. Recompilation will be
triggered by `stale_cachefile()` if that absolute path does not exist.
Previously this caused any `@depot` tags to be not resolved and so
trigger recompilation.
2. (new behavior) If we can't find a depot for a relocatable path, we
still replace it with the depot we found from other files. Recompilation
will be triggered by `stale_cachefile()` because the resolved path does
not exist.
3. (this behavior is kept) We require that relocatable paths all resolve
to the same depot.
4. (new behavior) We no longer use the first matching depot for
replacement, but instead we explicitly check that all resolve to the
same depot. This has two reasons:
- We want to scan all source files anyways in order to provide logs for
1. and 2. above, so the check is free.
- It is possible that a depot might be missing source files. Assume that
we have two depots on `DEPOT_PATH`, `depot_complete` and
`depot_incomplete`.
If `DEPOT_PATH=["depot_complete","depot_incomplete"]` then no
recompilation shall happen, because `depot_complete` will be picked.
If `DEPOT_PATH=["depot_incomplete","depot_complete"]` we trigger
recompilation and hopefully a meaningful error about missing files is
thrown. If we were to just select the first depot we find, then whether
recompilation happens would depend on whether the first relocatable file
resolves to `depot_complete` or `depot_incomplete`.
@KristofferC
Copy link
Member Author

I had forgotten the uuid entry in my Project.toml for Foo.

KristofferC pushed a commit that referenced this issue Apr 9, 2024
…files (#53905)

Fixes
#53859 (comment),
which was actually fixed before in
#52346, but
#52750 undid that fix.

This PR does not directly address #53859, because I can not reproduce it
atm.

---

The `@depot` resolution logic for `include()` files is adjusted as
follows:

1. (new behavior) If the cache is not relocatable because of an absolute
path, we ignore that path for the depot search. Recompilation will be
triggered by `stale_cachefile()` if that absolute path does not exist.
Previously this caused any `@depot` tags to be not resolved and so
trigger recompilation.
2. (new behavior) If we can't find a depot for a relocatable path, we
still replace it with the depot we found from other files. Recompilation
will be triggered by `stale_cachefile()` because the resolved path does
not exist.
3. (this behavior is kept) We require that relocatable paths all resolve
to the same depot.
4. (new behavior) We no longer use the first matching depot for
replacement, but instead we explicitly check that all resolve to the
same depot. This has two reasons:
- We want to scan all source files anyways in order to provide logs for
1. and 2. above, so the check is free.
- It is possible that a depot might be missing source files. Assume that
we have two depots on `DEPOT_PATH`, `depot_complete` and
`depot_incomplete`.
If `DEPOT_PATH=["depot_complete","depot_incomplete"]` then no
recompilation shall happen, because `depot_complete` will be picked.
If `DEPOT_PATH=["depot_incomplete","depot_complete"]` we trigger
recompilation and hopefully a meaningful error about missing files is
thrown. If we were to just select the first depot we find, then whether
recompilation happens would depend on whether the first relocatable file
resolves to `depot_complete` or `depot_incomplete`.

(cherry picked from commit d8d3842)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:precompilation Precompilation of modules regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants