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

Faster startup #79

Closed
wants to merge 1 commit into from
Closed

Conversation

PallHaraldsson
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #79 (844ba7b) into master (bd775f9) will decrease coverage by 11.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #79       +/-   ##
==========================================
- Coverage   17.15%   6.07%   -11.09%     
==========================================
  Files          71      71               
  Lines        4285    4299       +14     
==========================================
- Hits          735     261      -474     
- Misses       3550    4038      +488     
Impacted Files Coverage Δ
src/PythonCall.jl 0.00% <ø> (-76.48%) ⬇️
src/concrete/set.jl 0.00% <0.00%> (-100.00%) ⬇️
src/concrete/dict.jl 0.00% <0.00%> (-100.00%) ⬇️
src/concrete/list.jl 0.00% <0.00%> (-100.00%) ⬇️
src/concrete/consts.jl 0.00% <0.00%> (-100.00%) ⬇️
src/concrete/import.jl 0.00% <0.00%> (-100.00%) ⬇️
src/abstract/object.jl 2.17% <0.00%> (-92.66%) ⬇️
src/compat/stdlib.jl 0.00% <0.00%> (-90.00%) ⬇️
src/concrete/tuple.jl 3.22% <0.00%> (-87.10%) ⬇️
src/concrete/bytes.jl 0.00% <0.00%> (-77.28%) ⬇️
... and 39 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd775f9...844ba7b. Read the comment docs.

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Dec 6, 2021

You could rather do @eval Base.Experimental.@optlevel 1 if you want to be concervative, as done at PyCall.jl (and they've still not merged at https://github.com/JuliaInterop/RCall.jl/pull/383/files)

This takes 1.37 sec. or 31% off the time you have:

julia> @time using PythonCall
  4.431433 seconds (2.88 M allocations: 164.448 MiB, 1.13% gc time, 82.64% compilation time)

which is already a big improvement over your previous numbers. I like what you've done with the package, and downloading Julia or Python depending on where calling from. I see this could be my go-to package. Could should it rather download Python 3.10 (or 3.9 if 3.10 problematic) now by default. I've not yet looked, there may be an option to not download, use a specific, previously installed version?

And you can run the code in the PR with the new option (I think effectively disabling it):

$ ~/julia-1.7.0/bin/julia --min-optlevel=2
help?> Base.Experimental.@optlevel

The effective optimization level is the minimum of that specified on the command line and in per-module settings. If a --min-optlevel value is set on the command line, that is enforced as a lower bound.

That does at least something even if the description is for the older @optlevel not the newer variant I used. But the interpreter may still be on, not sure.

See also: SciML/diffeqpy#97

@cjdoris
Copy link
Collaborator

cjdoris commented Dec 15, 2021

Thanks for the PR. I'm hesitant to do something like this because it's reducing the startup time at the expense of making everything a bit slower afterwards. If this slow-down is negligible then fine, but I don't have the time at the moment to investigate if this is the case.

By default PythonCall should download the latest version of Python. Except that due to a bug in Conda, it is currently pinned to 3.9 or less, but this restriction should go away soon. You can use a preinstalled Python by setting the environment variable JULIA_PYTHONCALL_EXE to its path, but you lose the automatic package management if you do that.

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Dec 15, 2021

I understand you hesitation, and it may be warranted. You could merge on master first as is, to get people to test... or change to -O1 as done at PyCall:

https://github.com/JuliaPy/PyCall.jl/blob/master/src/PyCall.jl

i.e. without --compile=min, I may have gone overboard there, as it triggers the interpreter.

EDIT: Also something to keep track of: JuliaLang/julia#43370

@cjdoris
Copy link
Collaborator

cjdoris commented Feb 17, 2022

I did something like this on the dependencies MicroMamba and CondaPkg a while ago. It brought the load time of PythonCall down quite a bit. I don't want to apply the same trick to the whole of PythonCall though right now because it will slow things down at run time. Thanks for the tip about @max_methods!

@cjdoris cjdoris closed this Feb 17, 2022
@PallHaraldsson PallHaraldsson deleted the patch-1 branch February 17, 2022 23:50
@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Feb 18, 2022

EDIT: To answer my issue here, just getting red of (or to be safe, for now moving) worked as I though it should:

shell> mv ~/.julia/dev/PythonCall/ ~/.julia/dev/PythonCall_old/

then: dev PythonCall

I guess "free" isn't supposed to work, to not lose your changes, unless you made none, so you need to figure out above yourself.

I'm trying to help more, but I get a downgrade:

(@v1.7) pkg> dev PythonCall
   Resolving package versions...
    Updating `~/.julia/environments/v1.7/Project.toml`
  [6099a3de] ~ PythonCall v0.6.0 ⇒ v0.4.3 `~/.julia/dev/PythonCall`

I'm not sure this is usual, free didn't work (as I think it should), or just very unusual, e.g. with your package. I see similar here (by no answer):

https://discourse.julialang.org/t/pkg-develop-causing-package-to-downgrade/41484

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.

2 participants