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

add an LRU cache to precompile files #32651

Merged
merged 1 commit into from
Aug 16, 2019
Merged

add an LRU cache to precompile files #32651

merged 1 commit into from
Aug 16, 2019

Conversation

KristofferC
Copy link
Sponsor Member

instead of always clobbering the one precompile file per package for multiple environments, introduce a LRU cache (maximum of, currently, 10 files).

Fixes #27418

@KristofferC KristofferC added the packages Package management and loading label Jul 22, 2019
@KristofferC KristofferC requested a review from vtjnash July 22, 2019 00:20
@KristofferC
Copy link
Sponsor Member Author

cc @tkf who have been thinking about these stuff as well

@KristofferC
Copy link
Sponsor Member Author

Thinking about this a little more the implementation in this PR is non ideal because iteratively working on a project for a while will quickly clobber the whole cache of precompilation files.

We need to get the path of the current active project in there. Will amend this PR.

base/loading.jl Outdated Show resolved Hide resolved
@tkf
Copy link
Member

tkf commented Jul 22, 2019

We need to get the path of the current active project in there.

I think the tricky part is supporting switching projects. If you interleave Pkg.activates with imports, the "effective manifest" of the current process would be hard to re-construct in precompilation subprocesses.

I think an easier solution would be to use Base.HOME_PROJECT[] as a salt in package_slug. That is to say, we only guarantee that julia --project A and julia --project B have separate precompilation file set. Starting a session with julia and then pkg> activate A or pkg> activate B will still clobber the precompilation files.

base/loading.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC force-pushed the kc/lru_precompile branch 6 times, most recently from 5ae65b8 to f43434c Compare July 26, 2019 15:47
@tkf
Copy link
Member

tkf commented Jul 26, 2019

Re early my comment #32651 (comment)
I'm now convinced that my concern is invalid. This PR seems to be a very clever solution!

instead of always clobbering the one precompile file per package for multiple environments, introduce a LRU cache (10 files that we cycle through).
@andreasnoack
Copy link
Member

andreasnoack commented Aug 16, 2019

Let's merge this by the end of the day unless somebody speaks up. This is a great improvement over the current situation so it would be good to have included in 1.3 even if we want to adjust the details later.

@andreasnoack andreasnoack merged commit 694d59a into master Aug 16, 2019
@delete-merged-branch delete-merged-branch bot deleted the kc/lru_precompile branch August 16, 2019 18:08
marius311 pushed a commit to marius311/julia that referenced this pull request Aug 20, 2019
instead of always clobbering the one precompile file per package for multiple environments, introduce a LRU cache (10 files that we cycle through).
@bjarthur
Copy link
Contributor

less clobbering of precompilation files when changing environments is the #1 reason i'd upgrade to 1.3. unless i missed it, this PR is not mentioned in HISTORY.md. worth adding i think.

@ararslan ararslan added the needs news A NEWS entry is required for this change label Oct 28, 2019
bjarthur added a commit to bjarthur/julia that referenced this pull request Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs news A NEWS entry is required for this change packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How precompile files are loaded need to change if using multiple projects are going to be pleasant
6 participants