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 to improve package load time #55

Closed
carstenbauer opened this issue Jan 28, 2023 · 9 comments
Closed

Try to improve package load time #55

carstenbauer opened this issue Jan 28, 2023 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@carstenbauer
Copy link
Owner

Apart from initial attempts to use SnoopPrecompile we haven't yet spent much/any time on optimizing the load time. Apart from using general strategies (like avoiding invalidations etc.) we should try to benchmark and improve __init__() and _create_sysinfo_obj in particular.

FWIW, also citing @giordano from Discord:

also for the record loading time improved quite a lot in v1.9

giordano@ln04:~/software> julia-1.8.5/bin/julia -q
julia> @time using ThreadPinning
  3.089130 seconds (878.40 k allocations: 55.383 MiB, 87.15% compilation time)

julia> 
giordano@ln04:~/software> julia-1.9.0-beta3/bin/julia -q
julia> @time using ThreadPinning
  0.573002 seconds (467.70 k allocations: 32.458 MiB, 36.22% compilation time)
@carstenbauer carstenbauer added enhancement New feature or request good first issue Good for newcomers labels Jan 28, 2023
@giordano
Copy link

giordano commented Jan 29, 2023

Compilation latency of update_sysinfo! in Julia v1.8 looks quite slow. I'm not sure in

_lscpu2table(lscpustr = nothing)::Union{Nothing, Matrix{String}} = readdlm(IOBuffer(lscpustr),
String)
it makes much sense to have lscpustr=nothing and annotate the output as Union{Nothing, Matrix{String}}:

  • IOBuffer(::Nothing) is undefined
  • how can you get nothing as output, also given the previous point?

However

diff --git a/src/sysinfo.jl b/src/sysinfo.jl
index e070dc4..4e5b0a7 100644
--- a/src/sysinfo.jl
+++ b/src/sysinfo.jl
@@ -88,8 +88,8 @@ function lscpu2sysinfo(lscpustr = nothing)
     return sysinfo
 end
 
-_lscpu2table(lscpustr = nothing)::Union{Nothing, Matrix{String}} = readdlm(IOBuffer(lscpustr),
-                                                                           String)
+_lscpu2table(lscpustr::AbstractString) = readdlm(IOBuffer(lscpustr),
+                                                 String)::Matrix{String}
 
 function _lscpu_table_to_columns(table)::NamedTuple{(:idcs, :cpuid, :socket, :numa, :core),
                                                     NTuple{5, Vector{Int}}}

doesn't make any difference in terms of compilation latency of update_sysinfo!, so this isn't the real culprit here, but might be something that could be changed for consistency.

ThreadPinning._create_sysinfo_obj seems to have some type-unstable variables inside its body, although the return value of the function itself is fully stable.

@carstenbauer
Copy link
Owner Author

After the revamp we have

julia> @time_imports using ThreadPinning
      0.6 ms  StableTasks
      9.5 ms  Preferences
      0.3 ms  PrecompileTools
     11.6 ms  ThreadPinningCore
      0.4 ms  JLLWrappers
               ┌ 3.3 ms Hwloc_jll.__init__() 81.11% compilation time
     35.0 ms  Hwloc_jll 96.70% compilation time
               ┌ 4.6 ms SuiteSparse_jll.__init__()
      5.0 ms  SuiteSparse_jll
               ┌ 9.6 ms SparseArrays.CHOLMOD.__init__() 70.84% compilation time
    148.6 ms  SparseArrays 4.56% compilation time
      0.9 ms  Statistics
      1.3 ms  CEnum
     14.3 ms  Hwloc
      0.5 ms  DelimitedFiles
     40.6 ms  SysInfo
     44.4 ms  ThreadPinning

Pretty unfortunate that most of the time is spent with SparseArrays which we don't use at all here...

@giordano
Copy link

giordano commented Aug 5, 2024

Which package is pulling it in? Statistics?

@carstenbauer
Copy link
Owner Author

Looks like it.

julia> PkgDependency.tree("ThreadPinning")
ThreadPinning v1.0.0
  ├─ StableTasks v0.1.5
  ├─ PrecompileTools v1.2.1
  │  └─ Preferences v1.4.3
  │     └─ TOML v1.0.3
  ├─ DelimitedFiles v1.9.1
  ├─ ThreadPinningCore v0.4.5
  │  ├─ StableTasks v0.1.5 (*)
  │  └─ PrecompileTools v1.2.1 (*)
  ├─ SysInfo v0.2.5
  │  ├─ PrecompileTools v1.2.1 (*)
  │  ├─ DelimitedFiles v1.9.1 (*)
  │  └─ Hwloc v3.2.0
  │     ├─ Statistics v1.10.0
  │     │  └─ SparseArrays v1.10.0
  │     │     └─ SuiteSparse_jll v7.2.1+1
  │     │        └─ libblastrampoline_jll v5.8.0+1
  │     ├─ CEnum v0.5.0
  │     └─ Hwloc_jll v2.11.1+0
  │        └─ JLLWrappers v1.5.0
  │           └─ Preferences v1.4.3 (*)
  ├─ DocStringExtensions v0.9.3
  └─ Preferences v1.4.3 (*)

@giordano
Copy link

giordano commented Aug 5, 2024

As far as I understand from JuliaStats/Statistics.jl#134, SparseArrays.jl should be a weak dependency

@carstenbauer
Copy link
Owner Author

Seems like Statistics is an unused dependency of Hwloc. I'm dropping it in JuliaRegistries/General#112446.

@carstenbauer
Copy link
Owner Author

Now we have:

julia> @time_imports using ThreadPinning
      0.9 ms  StableTasks
     15.4 ms  Preferences
      0.3 ms  PrecompileTools
     17.5 ms  ThreadPinningCore
      0.4 ms  JLLWrappers
               ┌ 3.7 ms Hwloc_jll.__init__() 74.07% compilation time
     37.4 ms  Hwloc_jll 91.98% compilation time
      1.8 ms  CEnum
     16.4 ms  Hwloc
      0.5 ms  DelimitedFiles
     47.7 ms  SysInfo
     49.8 ms  ThreadPinning

@carstenbauer
Copy link
Owner Author

On current main branch (after the revamp):

julia> @time using ThreadPinning
  0.147620 seconds (122.75 k allocations: 11.089 MiB, 2.67% compilation time)

julia> @time pinthreads(:cores)
  0.187107 seconds (10.22 k allocations: 959.922 KiB, 3.71% compilation time)

julia> @time pinthreads(:cores)
  0.000705 seconds (81 allocations: 19.508 KiB)

Current stable release (before the revamp):

julia> @time using ThreadPinning
  0.349155 seconds (182.22 k allocations: 15.247 MiB, 42.82% compilation time)

julia> @time pinthreads(:cores)
  0.000967 seconds (832 allocations: 653.484 KiB)

julia> @time pinthreads(:cores)
  0.000518 seconds (824 allocations: 653.031 KiB)

@giordano
Copy link

giordano commented Aug 5, 2024

As far as I understand from JuliaStats/Statistics.jl#134, SparseArrays.jl should be a weak dependency

Ah, that's only in Julia v1.11, that's probably why you we're seeing SparseArrays still being loaded. But good to remove from the stack a useless dependency anyway!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants