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

REPL start time fix #20

Merged
merged 5 commits into from
Dec 13, 2019
Merged

Conversation

daviehh
Copy link
Contributor

@daviehh daviehh commented Nov 30, 2019

Can fix the issue #1

Stock Julia without any packages:

> time julia -e ''
0.12s user 0.11s system 65% cpu 0.356 total

> time julia -e 'import Pkg; Pkg.status()'
0.22s user 0.12s system 89% cpu 0.379 total

MKL.jl

> time julia -e ''
1.07s user 0.18s system 117% cpu 1.069 total

> time julia -e 'import Pkg; Pkg.status()'
5.49s user 0.25s system 102% cpu 5.616 total

Fixed (after 1st run):

> time julia -e ''
0.13s user 0.11s system 66% cpu 0.368 total

> time julia -e 'import Pkg; Pkg.status()'
0.27s user 0.13s system 96% cpu 0.416 total

Also added current julia version (1.3) to travis test.

Note that according to the discussion in #1 one should generate precompile statements during the generation of sysimg, not after like here, but just replacing force_native_image!() with PackageCompiler.build_sysimg(PackageCompiler.default_sysimg_path(), "generate_precompile.jl") does not work on my end...(does not replace :mkl https://travis-ci.com/daviehh/MKL.jl/builds/138789653)

@aminya
Copy link
Contributor

aminya commented Nov 30, 2019

This is awesome. 🤓👌🏻

@Jutho Jutho mentioned this pull request Dec 6, 2019
@Jutho
Copy link

Jutho commented Dec 7, 2019

Great! Confirmed to work on julia 1.2 and 1.3.

deps/fix_repl.jl Outdated Show resolved Hide resolved
@andreasnoack
Copy link
Member

@staticfloat Do you know what the Windows error is?

@staticfloat
Copy link
Member

It's fixed by BioJulia/Libz.jl@c82db8e but it needs a new version of Libz.jl to be tagged.

@andreasnoack
Copy link
Member

I think this is ready but I'd like to see tests passing on Windows. Hopefully, there'll be a Libz.jl release fairly soon.

@daviehh
Copy link
Contributor Author

daviehh commented Dec 12, 2019

@andreasnoack Thanks! With the new Libz.jl released, I did a test on my fork for windows (I do not have access to windows machines locally so only tested on appveyor), now it no longer complains about libz but errors at 7z.exe when building PackageCompiler...

https://ci.appveyor.com/project/daviehh/mkl-jl/builds/29505379/job/3bth9m9vnka5y8bs#L93

Looks similar to error message on PackageCompiler's end:
https://ci.appveyor.com/project/SimonDanisch/packagecompiler-jl-otum8/builds/28206590/job/bxfctsqln2ajkcx3#L97

@andreasnoack
Copy link
Member

@staticfloat Do you know what the issue with 7z is?

@staticfloat
Copy link
Member

PackageCompiler is not looking in the right place for 7z.exe on 1.3, since it got moved: it needs to do something like:

searchpath = ENV["PATH"]
if isdefined(Base, :LIBEXECDIR)
	sepchar = Sys.iswindows() ? ";" : ":"
	searchpath = string(joinpath(Sys.BINDIR, Base.LIBEXECDIR), sepchar, searchpath)
end
7z_path = withenv("PATH" => searchpath) do
	Sys.which(`7z`)
end
run(`\$7z_path ...`)

@daviehh
Copy link
Contributor Author

daviehh commented Dec 13, 2019

@andreasnoack
Copy link
Member

Okay. Unfortunately, it appears that Windows is currently not supported so let's get this one merged. We should probably switch to https://github.com/KristofferC/PackageCompilerX.jl once it has been released.

@andreasnoack andreasnoack merged commit 06b2dff into JuliaLinearAlgebra:master Dec 13, 2019
@andreasnoack
Copy link
Member

I might hold off a bit with a new release since it might trigger a new build and therefore potentially break systems that are currently working.

@aminya
Copy link
Contributor

aminya commented Dec 13, 2019

We should probably switch to https://github.com/KristofferC/PackageCompilerX.jl once it has been released.

Fixing the Windows problem on PackageCompiler is a faster solution than switching to a different system. From what I remember, PackageCompilerX is not supported on Windows yet.

@KristofferC
Copy link
Contributor

PackageCompilerX is not supported on Windows yet.

Not true, it even has Windows CI.

@daviehh
Copy link
Contributor Author

daviehh commented Dec 13, 2019

I have tested PackageCompilerX on mac and linux, it worked great for MKL.jl here, it should be easy to switch when PackageCompilerX is released. Also tested with JuliaLang/julia#30746 PR where the script saved precompile statements that PackageCompilerX can directly use.

@aminya
Copy link
Contributor

aminya commented Dec 13, 2019

PackageCompilerX is not supported on Windows yet.

Not true, it even has Windows CI.

Thanks for clarification. I think I misunderstood some of your messages on Slack.

@KristofferC
Copy link
Contributor

Thanks for clarification. I think I misunderstood some of your messages on Slack.

Maybe, or the state of things changed since I wrote them :P

@aminya
Copy link
Contributor

aminya commented Dec 14, 2019

The Windows problem should be fixed once JuliaRegistries/General#6375 and JuliaLang/PackageCompiler.jl#294 are merged.

@aminya
Copy link
Contributor

aminya commented Dec 19, 2019

@andreasnoack You can tag a new release for MKL. I added a PR for V0.1 tagging: #22

New version of PkgCompiler is merged. This should solve the issue on Windows.
JuliaRegistries/General#6848

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.

None yet

6 participants