MPI support via package extensions (weak dependency) #1062

ranocha · 2023-04-21T14:54:40Z

This setup keeps using Requires.jl for Julia v1.8 and older but switches to package extensions for Julia v1.9.

With Julia v1.8.5, I get (for both this PR and the current release of HDF5.jl)

julia> @time_imports using MPI, HDF5
      0.2 ms  Requires
      2.1 ms  DocStringExtensions 56.69% compilation time
     19.0 ms  Preferences
      0.2 ms  MPIPreferences
      0.2 ms  JLLWrappers
      0.3 ms  CompilerSupportLibraries_jll
      2.9 ms  MPICH_jll 66.09% compilation time
    208.3 ms  MPI 69.33% compilation time (<1% recompilation)
      0.6 ms  Compat
      0.1 ms  Zlib_jll
      0.5 ms  OpenSSL_jll
      0.6 ms  HDF5_jll
    631.2 ms  HDF5 88.36% compilation time (63% recompilation)

With Julia v1.9.0-rc1 and the current release of HDF5.jl, I get

julia> @time_imports using MPI, HDF5
      0.3 ms  Requires
      1.1 ms  DocStringExtensions
      9.4 ms  Preferences
      0.3 ms  MPIPreferences
      0.2 ms  JLLWrappers
      3.4 ms  MPICH_jll 74.32% compilation time
    167.6 ms  MPI 88.40% compilation time
      0.1 ms  Compat
      0.1 ms  Compat → CompatLinearAlgebraExt
      0.1 ms  Zlib_jll
      0.5 ms  OpenSSL_jll
      0.7 ms  HDF5_jll
    149.7 ms  HDF5 62.28% compilation time

With Julia v1.9.0-rc1 and this PR, I get

julia> @time_imports using MPI, HDF5
      0.5 ms  Requires
      1.6 ms  DocStringExtensions
     14.0 ms  Preferences
      0.3 ms  MPIPreferences
      0.2 ms  JLLWrappers
      3.8 ms  MPICH_jll 71.64% compilation time
    160.6 ms  MPI 87.36% compilation time
      0.1 ms  Compat
      0.1 ms  Compat → CompatLinearAlgebraExt
      0.1 ms  Zlib_jll
      0.5 ms  OpenSSL_jll
      0.8 ms  HDF5_jll
     46.6 ms  HDF5
      0.4 ms  HDF5 → MPIExt

This avoids some compilation time whenever MPI and HDF5 are loaded together.

ranocha · 2023-04-21T15:13:25Z

There is a problem with the current approach to define HDF5.Drivers.MPIO depending on whether MPI.jl has been loaded. Citing Kristoffer Carlsson from discourse:

Extensions are a run time thing, they might be loaded or they might not be so you have to check. Therefore, you cannot make a decision about them during precompile time (or macro expansion time). That’s why you need to run some code (like a function) to interact with them and you cannot really just export a type from it.

Just exporting a type from it is basically the current approach HDF5.jl takes with MPIO. Any preferences how to fix this?

ranocha · 2023-04-21T15:38:06Z

I implemented a workaround for the issue in 30cf866. I am not completely happy with it, but it is the best I can think of right now to keep compatibility with Julia v1.6 - v1.8.

ranocha · 2023-04-21T15:38:09Z

Package extensions are only supported in Julia v1.6 and newer. Since this compat bump is included in #1061, we can wait until the other PR is merged before continuing with this PR.

mkitti · 2023-04-21T18:10:50Z

This should coordinate with JuliaPackaging/Yggdrasil#6551

eschnett · 2023-04-21T20:14:15Z

@mkitti What exactly need to be coordinated?

mkitti · 2023-04-21T21:10:47Z

@eschnett As far as I know, we have never loaded a MPI-enabled HDF5_jll. For this package extension to work properly, we should examine if the loaded JLL package is actually MPI capable.

ranocha · 2023-04-24T05:50:58Z

As far as I understand, the current approach of HDF5.jl is as follows

Load the MPI code only if MPI.jl is loaded
If the MPI code is loaded and called with an HDF5 implementation not supporting MPI, throw an error

That's still the same approach when rewriting it as a package extension. Thus, I am not sure I understand. Do you suggest to wait until JuliaPackaging/Yggdrasil#6551 and #1061 are merged before merging this PR?

mkitti · 2023-04-24T19:18:05Z

The first issue is awareness that both issues are coming down the pipeline.

We should now be able to test versus the new HDF5_jlls.

The problem here is that we are changing multiple components at once here, so we need to make sure to do integrated testing to ensure that changing either component alone or changing both does not create unexpected issues. Otherwise, it will get complicated to diagnose future problems.

ranocha · 2023-04-25T03:58:01Z

Ok. I will leave this PR open. Please let me know when you think it's a good time to revisit it

mkitti · 2023-04-25T04:22:10Z

I'm mainly just triaging here. It's @simonbyrne that is really needed to review this carefully.

src/drivers/drivers.jl

simonbyrne · 2023-04-28T05:33:58Z

Should we just ditch Julia 1.3 support at this point? MPI.jl has a minimum Julia version of 1.6 (the current LTS)

ext/MPIExt.jl

ranocha · 2023-04-28T17:16:53Z

Should we just ditch Julia 1.3 support at this point? MPI.jl has a minimum Julia version of 1.6 (the current LTS)

Yes, I think this would be reasonable. This compat bump is included in #1061. Thus, I think we can wait until the other PR is merged before continuing with this PR.

KristofferC · 2023-05-29T14:14:12Z

Bump, HDF5 has the largest init time out of a large set of packages in something I profile:

ranocha · 2023-05-29T16:13:13Z

We're just waiting for #1061 to be merged since it updates the minimum Julia version and the CI tests accordingly.

mkitti · 2023-05-29T18:55:12Z

Bump, HDF5 has the largest init time out of a large set of packages in something I profile:

Interesting. Could you see if this is true for HDF5 v0.16.14 and/or HDF5_jll.

I do suspect this is MPI related.

KristofferC · 2023-05-29T19:07:23Z

Interesting. Could you see if this is true for HDF5 v0.16.14 and/or HDF5_jll.

Not sure what this means exactly but the screenshot is for __init__ in the HDF5.Drivers submodule.

ranocha · 2023-05-29T19:13:26Z

Bump, HDF5 has the largest init time out of a large set of packages in something I profile:

Interesting. Could you see if this is true for HDF5 v0.16.14 and/or HDF5_jll.

I do suspect this is MPI related.

Yes, it's caused by the way how MPI is handled at the moment (I assume it's OmniPackage.jl depending on Trixi.jl depending on both MPI.jl and HDF5.jl)

mkitti · 2023-05-29T22:43:24Z

I suspect that the increase in driver load time is a recent regression. The new HDF5_jll builds make MPI always available, so we may loading the driver every time.

mkitti · 2023-05-29T23:19:16Z

I'm having some trouble reproducing, but perhaps I am missing something.

julia> @time_imports using MPI, HDF5
      4.0 ms  DocStringExtensions
     31.6 ms  Preferences
      0.5 ms  MPIPreferences
      0.5 ms  JLLWrappers
      6.8 ms  MPICH_jll 70.80% compilation time
      0.3 ms  PrecompileTools
    128.3 ms  MPI 58.11% compilation time
      0.3 ms  Requires
      0.1 ms  Compat
      0.2 ms  Compat → CompatLinearAlgebraExt
      0.9 ms  OpenSSL_jll
      0.2 ms  Zlib_jll
      0.4 ms  libaec_jll
      4.0 ms  HDF5_jll
    268.3 ms  HDF5 66.09% compilation time

(jl_XytuBr) pkg> st
Status `/tmp/jl_XytuBr/Project.toml`
  [f67ccb44] HDF5 v0.16.15
  [da04e1cc] MPI v0.20.9

(jl_XytuBr) pkg> st -m
Status `/tmp/jl_XytuBr/Manifest.toml`
  [34da2185] Compat v4.6.1
  [ffbed154] DocStringExtensions v0.9.3
  [f67ccb44] HDF5 v0.16.15
  [692b3bcd] JLLWrappers v1.4.1
  [da04e1cc] MPI v0.20.9
  [3da0fdf6] MPIPreferences v0.1.7
  [aea7be01] PrecompileTools v1.1.1
  [21216c6a] Preferences v1.4.0
  [ae029012] Requires v1.3.0
  [0234f1f7] HDF5_jll v1.14.0+0
  [1d63c593] LLVMOpenMP_jll v15.0.4+0
  [7cb0a576] MPICH_jll v4.1.1+1
  [f1f71cc9] MPItrampoline_jll v5.3.0+0
  [9237b28f] MicrosoftMPI_jll v10.1.3+4
  [fe0851c0] OpenMPI_jll v4.1.5+0
  [458c3c95] OpenSSL_jll v3.0.8+0
  [477f73a3] libaec_jll v1.0.6+1
  [0dad84c5] ArgTools v1.1.1
  [56f22d72] Artifacts
  [2a0f44e3] Base64
  [ade2ca70] Dates
  [8ba89e20] Distributed
  [f43a241f] Downloads v1.6.0
  [7b1f6079] FileWatching
  [b77e0a4c] InteractiveUtils
  [4af54fe1] LazyArtifacts
  [b27032c2] LibCURL v0.6.3
  [76f85450] LibGit2
  [8f399da3] Libdl
  [56ddb016] Logging
  [d6f4376e] Markdown
  [a63ad114] Mmap
  [ca575930] NetworkOptions v1.2.0
  [44cfe95a] Pkg v1.9.0
  [de0858da] Printf
  [3fa0cd96] REPL
  [9a3f8284] Random
  [ea8e919c] SHA v0.7.0
  [9e88b42a] Serialization
  [6462fe0b] Sockets
  [fa267f1f] TOML v1.0.3
  [a4e569a6] Tar v1.10.0
  [cf7118a7] UUIDs
  [4ec0a83e] Unicode
  [e66e0078] CompilerSupportLibraries_jll v1.0.2+0
  [deac9b47] LibCURL_jll v7.84.0+0
  [29816b5a] LibSSH2_jll v1.10.2+0
  [c8ffd9c3] MbedTLS_jll v2.28.2+0
  [14a3606d] MozillaCACerts_jll v2022.10.11
  [83775a58] Zlib_jll v1.2.13+0
  [8e850ede] nghttp2_jll v1.48.0+0
  [3f19e933] p7zip_jll v17.4.0+0

mkitti · 2023-05-29T23:19:52Z

Could we get CI to pass?

ranocha · 2023-05-30T04:26:10Z

CI fails on Julia v1.3 since Preferencesjl requires Julia v1.6. Everything else passes here. I didn't want to update the lower bound on Julia here since that's already done in #1061

KristofferC · 2023-05-30T06:52:55Z

I'm having some trouble reproducing, but perhaps I am missing something.

It's on JuliaComputing/OmniPackage.jl#67.

ranocha · 2023-06-02T10:39:47Z

@mkitti I think this is ready for a review

mkitti · 2023-06-02T10:42:47Z

What is going on with the nightly CI failure? I'll take a closer look this weekend.

mkitti · 2023-06-03T02:34:40Z

@simonbyrne , would you like another look?

simonbyrne

Looks good to me!

ranocha · 2023-06-05T04:00:42Z

Great, thanks!

MPI support via package extensions (weak dependency)

57e65a2

workaround for Drivers.MPIO

30cf866

ranocha force-pushed the hr/mpi_extension branch from fd590b3 to 30cf866 Compare April 21, 2023 15:44

mkitti requested a review from simonbyrne April 25, 2023 04:21

simonbyrne reviewed Apr 25, 2023

View reviewed changes

src/drivers/drivers.jl Show resolved Hide resolved

mkitti reviewed Apr 27, 2023

View reviewed changes

src/drivers/drivers.jl Show resolved Hide resolved

mkitti added this to the 0.17.0 milestone Apr 27, 2023

Merge branch 'master' into hr/mpi_extension

adc7e85

simonbyrne reviewed Apr 28, 2023

View reviewed changes

ext/MPIExt.jl Show resolved Hide resolved

simonbyrne added 2 commits April 27, 2023 22:48

fix h5open docs

0d67e7f

apply formatter

124e6b1

display full diff in format check

224bf52

mkitti mentioned this pull request Apr 28, 2023

0.17 breaking changes #1064

Closed

ranocha added 2 commits June 2, 2023 11:55

Merge branch 'master' into hr/mpi_extension

31dd208

import Drivers.DRIVERS

7e78918

ranocha force-pushed the hr/mpi_extension branch from 22a0905 to 7e78918 Compare June 2, 2023 10:19

mkitti requested a review from simonbyrne June 3, 2023 02:32

merge Format-check.yml from master

8e2d8f6

simonbyrne approved these changes Jun 4, 2023

View reviewed changes

simonbyrne merged commit 4bb35f5 into JuliaIO:master Jun 4, 2023

ranocha deleted the hr/mpi_extension branch June 5, 2023 04:00

ranocha mentioned this pull request Oct 5, 2023

mention package extension for MPI #1122

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MPI support via package extensions (weak dependency) #1062

MPI support via package extensions (weak dependency) #1062

ranocha commented Apr 21, 2023

ranocha commented Apr 21, 2023 •

edited

Loading

ranocha commented Apr 21, 2023 •

edited

Loading

ranocha commented Apr 21, 2023

mkitti commented Apr 21, 2023

eschnett commented Apr 21, 2023

mkitti commented Apr 21, 2023

ranocha commented Apr 24, 2023

mkitti commented Apr 24, 2023

ranocha commented Apr 25, 2023

mkitti commented Apr 25, 2023

simonbyrne commented Apr 28, 2023

ranocha commented Apr 28, 2023

KristofferC commented May 29, 2023

ranocha commented May 29, 2023

mkitti commented May 29, 2023

KristofferC commented May 29, 2023 •

edited

Loading

ranocha commented May 29, 2023 •

edited

Loading

mkitti commented May 29, 2023

mkitti commented May 29, 2023

mkitti commented May 29, 2023

ranocha commented May 30, 2023

KristofferC commented May 30, 2023

ranocha commented Jun 2, 2023

mkitti commented Jun 2, 2023

mkitti commented Jun 3, 2023

simonbyrne left a comment

ranocha commented Jun 5, 2023

MPI support via package extensions (weak dependency) #1062

MPI support via package extensions (weak dependency) #1062

Conversation

ranocha commented Apr 21, 2023

ranocha commented Apr 21, 2023 • edited Loading

ranocha commented Apr 21, 2023 • edited Loading

ranocha commented Apr 21, 2023

mkitti commented Apr 21, 2023

eschnett commented Apr 21, 2023

mkitti commented Apr 21, 2023

ranocha commented Apr 24, 2023

mkitti commented Apr 24, 2023

ranocha commented Apr 25, 2023

mkitti commented Apr 25, 2023

simonbyrne commented Apr 28, 2023

ranocha commented Apr 28, 2023

KristofferC commented May 29, 2023

ranocha commented May 29, 2023

mkitti commented May 29, 2023

KristofferC commented May 29, 2023 • edited Loading

ranocha commented May 29, 2023 • edited Loading

mkitti commented May 29, 2023

mkitti commented May 29, 2023

mkitti commented May 29, 2023

ranocha commented May 30, 2023

KristofferC commented May 30, 2023

ranocha commented Jun 2, 2023

mkitti commented Jun 2, 2023

mkitti commented Jun 3, 2023

simonbyrne left a comment

Choose a reason for hiding this comment

ranocha commented Jun 5, 2023

ranocha commented Apr 21, 2023 •

edited

Loading

ranocha commented Apr 21, 2023 •

edited

Loading

KristofferC commented May 29, 2023 •

edited

Loading

ranocha commented May 29, 2023 •

edited

Loading