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

loading: delete LOAD_CACHE_PATH #26165

Merged
merged 1 commit into from
Feb 23, 2018
Merged

Conversation

StefanKarpinski
Copy link
Member

No description provided.

@StefanKarpinski StefanKarpinski merged commit 33ca610 into master Feb 23, 2018
@StefanKarpinski StefanKarpinski deleted the sk/load_cache_path_begone branch February 23, 2018 03:08
pkg.uuid === nothing ? "$(pkg.name).ji" :
joinpath(pkg.name, "$(package_slug(pkg.uuid)).ji")
cache_file_entry(pkg::PkgId) = joinpath(
"compiled",
Copy link
Member

Choose a reason for hiding this comment

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

Why “compiled”?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this is where things that are compiled live. The option that controls whether julia creates or uses these files is called --compiled-modules. (There was also a bikeshed issue where I asked for comment on this, which was posted in multiple forums.)

@mbauman
Copy link
Member

mbauman commented Feb 23, 2018

When comparing the performance of two different julia builds (of the same $MAJOR.$MINOR), I found it exceedingly helpful to have two different LOAD_CACHE_PATHs. Otherwise, swapping between the two builds caused recompilation of the caches every single time… and accidentally running the usings simultaneously resulted in corrupted caches. So I had this in my juliarc:

if VERSION > v"0.7-"
    Base.LOAD_CACHE_PATH[1] = joinpath(homedir(), "packages", "lib-" * split(Sys.BINDIR, '/')[end-2])
end

That is — I'd use the path to the BINDIR to stash caches in different folders.

Edit to add: is there a way we can support this use-case?

@KristofferC
Copy link
Member

and accidentally running the usings simultaneously resulted in corrupted caches.

Seems we should have some lock files to prevent this?

@StefanKarpinski
Copy link
Member Author

I had talked with @vtjnash about arranging for ji files be write-once and doing away with all the invalidation logic. The pkg> gc command (which will be run automatically every now and then) would be responsible for cleaning up old ji files when the julia version and manifest that generated it no longer exist for a certain amount of time.

@vtjnash
Copy link
Member

vtjnash commented Feb 23, 2018

Seems we should have some lock files to prevent this?

https://github.com/vtjnash/Pidfile.jl, perhaps? 🙂 But needs someone to hook it up. And as Stefan said, we should additionally defer deleting them until an explicit "gc" step.

@tlnagy
Copy link
Contributor

tlnagy commented Mar 23, 2018

I'm working towards updating Compose to work with v0.7 (GiovineItalia/Compose.jl#279) and ran into this PR.

We throw an error message in Compose.jl that uses LOAD_CACHE_PATH to tell users to delete the precompiled version so that Pkg.build runs again:

https://github.com/GiovineItalia/Compose.jl/blob/7075ee7232331875a054d49177d46bc82e8ce75b/src/Compose.jl#L149-L152

msg2 = """
        You also have to delete $(joinpath(Base.LOAD_CACHE_PATH[1], "Compose.ji"))
        and restart your REPL session afterwards.
        """

How to we find this path without LOAD_CACHE_PATH?

EDIT: I see that it was renamed to DEPOT_PATH? So I guess Compat.jl will need to be updated to add this

@krinsman
Copy link

krinsman commented Aug 21, 2019

Is this change anywhere in the change logs?

I can't find it here: https://docs.julialang.org/en/v0.7.0/NEWS/

I am trying to update the startup.jl script for the conda-forge Julia release, and the tests do not seem to like the references to Base.LOAD_CACHE_PATH in the old/current startup.jl script.

Apparently it was replaced by DEPOT_PATH? So should I make a PR against the Julia changelog docs noting that LOAD_CACHE_PATH was removed and replaced with DEPOT_PATH?

@StefanKarpinski
Copy link
Member Author

We don't generally put changes to non-exported internals in NEWS. It has been deleted for over a year now—where would we put the NEWS at this point?

@krinsman
Copy link

Thank you for the very quick response! I appreciate it.

I know very little about Julia or its documentation, so I should have apologized for that in advance.

You're right, NEWS doesn't make sense. Wherever the changelog is for the version where this change was made would be more appropriate.

I don't know why anyone needed to call Base.LOAD_CACHE_PATH or now needs to call Base.DEPOT_PATH. I made the observation that someone's startup.jl did, and thus broke with this change, and the change is documented only on GitHub and not the official changelog documentation. Maybe that observation does not have any consequences; you are obviously in a better position to make that judgment than I am. I hoped making it might possibly help you.

@StefanKarpinski
Copy link
Member Author

Wherever the changelog is for the version where this change was made would be more appropriate.

The only changelog is the log of git commits between 0.6 and 1.0—which is enormous (over 6000 commits: there were a lot of changes in the lead up to 1.0). This change is in there: in the commit for this pull request. I'm not sure what to do here: Base.LOAD_CACHE_PATH was fairly advanced internal stuff—presumably anyone who was messing with it is a pretty advanced Julia user and has already figured out what happened and how to adjust. But in any case thanks for the heads up. Always better to err on the side of caution.

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

Successfully merging this pull request may close these issues.

6 participants