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

try run dynamic artifact selector in Pkg process #3714

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

KristofferC
Copy link
Sponsor Member

I don't really know what you are allowed to do in the dynamic artifact selector but this improves performance a lot at least (e.g. for Pkg.status) when packages using this are in the environment.

@giordano
Copy link
Contributor

CC @vchuravy

@KristofferC
Copy link
Sponsor Member Author

WARNING: Method definition augment_platform!(Base.BinaryPlatforms.Platform) in module Operations at /tmp/jl_OfJNuL/AugmentedPlatform/.pkg/platform_augmentation.jl:4 overwritten at /tmp/jl_oUdeKZ/AugmentedPlatform/.pkg/platform_augmentation.jl:4.

Probably needs to be evaluated in their own module.

@KristofferC
Copy link
Sponsor Member Author

Something has to be changed here!! I added a @time before the select_cmd command begin run and do a Pkg.resolve

julia>  Pkg.resolve()
  1.018608 seconds (820 allocations: 5.425 MiB)
  0.680233 seconds (199 allocations: 1.072 MiB, 7.64% gc time)
  0.593827 seconds (406 allocations: 2.523 MiB)
  0.965070 seconds (1.05 k allocations: 7.056 MiB)
  0.635199 seconds (622 allocations: 4.038 MiB)
  0.597117 seconds (964 allocations: 6.426 MiB)
  0.553596 seconds (54 allocations: 65.938 KiB)
  0.627717 seconds (955 allocations: 6.366 MiB)
  0.566144 seconds (54 allocations: 65.938 KiB)
  No Changes to `~/JuliaPkgs/OmniPackage.jl/Project.toml`
  No Changes to `~/JuliaPkgs/OmniPackage.jl/Manifest.toml`
  1.004013 seconds (1.32 k allocations: 8.941 MiB)
  0.605631 seconds (1.27 k allocations: 8.561 MiB, 1.11% gc time)
  0.623127 seconds (1.58 k allocations: 10.707 MiB)
  1.005323 seconds (1.53 k allocations: 10.394 MiB)
  0.674921 seconds (1.09 k allocations: 7.313 MiB, 0.61% gc time)
  0.578730 seconds (577 allocations: 3.722 MiB)
  0.585688 seconds (54 allocations: 65.938 KiB)
  0.628440 seconds (919 allocations: 6.121 MiB)
  0.565631 seconds (54 allocations: 65.938 KiB)

Completely unacceptable.

@IanButterworth
Copy link
Sponsor Member

IanButterworth commented May 9, 2024

Alternatively we could try running this process without pkgimages?

Update:

I tried this. Perhaps not fast enough.

(@v1.12) pkg> dev https://github.com/JuliaComputing/OmniPackage.jl
     Cloning git-repo `https://github.com/JuliaComputing/OmniPackage.jl`
   Resolving package versions...
  1.871987 seconds (95 allocations: 67.562 KiB, 0.12% compilation time: 100% of which was recompilation)
  0.292939 seconds (54 allocations: 65.938 KiB)
  0.331011 seconds (1.39 k allocations: 9.374 MiB, 2.22% gc time)
  0.294084 seconds (54 allocations: 65.938 KiB)
  0.319483 seconds (54 allocations: 65.938 KiB)
  0.372198 seconds (811 allocations: 5.344 MiB, 16.00% gc time)
  0.294497 seconds (54 allocations: 65.938 KiB)
  0.293367 seconds (54 allocations: 65.938 KiB)

@KristofferC KristofferC requested a review from a team as a code owner May 9, 2024 13:57
@vchuravy vchuravy requested a review from staticfloat May 9, 2024 14:37
@DilumAluthge DilumAluthge removed the request for review from a team May 9, 2024 15:01
@KristofferC
Copy link
Sponsor Member Author

KristofferC commented May 9, 2024

This is not so good (but can be trivially fixed):

julia> LOAD_PATH
57-element Vector{String}:
 "@"
 "@v#.#"
 "@stdlib"
 "/home/kc/.julia/packages/CUDA_Runtime_jll/bmudn"
 "/home/kc/.julia/packages/P4est_jll/kfjxs"
 "/home/kc/.julia/packages/t8code_jll/RBsof"
 "/home/kc/.julia/packages/XGBoost_jll/WE2Ec"
 "/home/kc/.julia/packages/HDF5_jll/pjOXH"
 "/home/kc/.julia/packages/MPICH_jll/NeFHN"
 "/home/kc/.julia/packages/OpenMPI_jll/sHrC3"
 "/home/kc/.julia/packages/LLVMExtra_jll/iFERN"
 "/home/kc/.julia/packages/MPItrampoline_jll/ftT1U"
 "/home/kc/.julia/packages/CUDA_Runtime_jll/bmudn"
 "/home/kc/.julia/packages/P4est_jll/kfjxs"
 "/home/kc/.julia/packages/t8code_jll/RBsof"
 "/home/kc/.julia/packages/XGBoost_jll/WE2Ec"
 "/home/kc/.julia/packages/HDF5_jll/pjOXH"
 "/home/kc/.julia/packages/MPICH_jll/NeFHN"

Also packages seem to load the jll in their script:

https://github.com/JuliaBinaryWrappers/CUDA_Runtime_jll.jl/blob/6fc9e752fc55edbe606d8dd3a66b65469c2fbffa/.pkg/platform_augmentation.jl#L95-L99

so this would load the jll in the current process.

Sigh, this is all kinds of bad.

@IanButterworth
Copy link
Sponsor Member

Parallelize the artifact collection above this level?

@KristofferC
Copy link
Sponsor Member Author

I'm just sad to see us compiling virtually the same code over and over again in new processes just to do a stauts call even if that is running in parallel.

Maybe disable the is_instantiated in the status check for jlls using this functionality? Do some caching. I dunno...

@vchuravy
Copy link
Member

vchuravy commented May 9, 2024

Yeah we really want to execute this code in a sandbox and after running it throw away all side-effects.

But code-loading is currently hard to isolate... If we could unload modules that would be lovely...

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.

4 participants