Skip to content

SnoopPrecompile common methods #371

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

Merged
merged 1 commit into from
Jan 25, 2023
Merged

SnoopPrecompile common methods #371

merged 1 commit into from
Jan 25, 2023

Conversation

chriselrod
Copy link
Collaborator

@chriselrod chriselrod commented Jan 24, 2023

With ArrayInterface(Core) master:

julia> intersect(methodinstances(cf_odeq), methodinstances(cf_rf))
Set{Core.MethodInstance} with 17 elements:
  MethodInstance for Base.IteratorSize(::Type{Base.OneTo{Int64}})
  MethodInstance for ArrayInterfaceCore.known_step(::Type{Base.OneTo{Int64}})
  MethodInstance for ArrayInterfaceCore.is_forwarding_wrapper(::Type{Base.OneTo{Int64}})
  MethodInstance for ArrayInterfaceCore.known_first(::Type{Base.OneTo{Int64}})
  MethodInstance for ArrayInterface._maybe_known_length(::Base.HasShape{1}, ::Type{Base.O
  MethodInstance for convert(::Type{Tuple{UnitRange{Int64}, UnitRange{Int64}}}, ::Tuple{U
  MethodInstance for Base.indexed_iterate(::Tuple{Tuple{Int64}, typeof(identity)}, ::Int6
  MethodInstance for Base._xfadjoint_unwrap(::Tuple{Int64})
  MethodInstance for ArrayInterface.known_length(::Type{Base.OneTo{Int64}})
  MethodInstance for ArrayInterfaceCore.known_last(::Type{Base.OneTo{Int64}})
  MethodInstance for Base.Generator{Tuple{Int64}, typeof(identity)}(::typeof(identity), :
  MethodInstance for ArrayInterface._prod_or_nothing(::Tuple{Nothing})
  MethodInstance for ArrayInterface.known_size(::Type{Base.OneTo{Int64}})
  MethodInstance for getproperty(::Base.Generator{Tuple{Int64}, typeof(identity)}, ::Symb
  MethodInstance for Base.Generator(::typeof(identity), ::Tuple{Int64})
  MethodInstance for Base.indexed_iterate(::Tuple{Tuple{Int64}, typeof(identity)}, ::Int6
  MethodInstance for Base._xfadjoint_unwrap(::Base.Generator{Tuple{Int64}, typeof(identit

With this PR:

julia> intersect(methodinstances(cf_odeq), methodinstances(cf_rf))
Set{Core.MethodInstance} with 8 elements:
  MethodInstance for Base.indexed_iterate(::Tuple{Tuple{Int64}, typeof(identity)}, ::Int6
  MethodInstance for Base.Generator(::typeof(identity), ::Tuple{Int64})
  MethodInstance for convert(::Type{Tuple{UnitRange{Int64}, UnitRange{Int64}}}, ::Tuple{U
  MethodInstance for Base.indexed_iterate(::Tuple{Tuple{Int64}, typeof(identity)}, ::Int6
  MethodInstance for Base._xfadjoint_unwrap(::Tuple{Int64})
  MethodInstance for Base._xfadjoint_unwrap(::Base.Generator{Tuple{Int64}, typeof(identit
  MethodInstance for getproperty(::Base.Generator{Tuple{Int64}, typeof(identity)}, ::Symb
  MethodInstance for Base.Generator{Tuple{Int64}, typeof(identity)}(::typeof(identity), :

@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Merging #371 (2d6e298) into master (6bfbcf7) will not change coverage.
The diff coverage is 93.75%.

@@           Coverage Diff           @@
##           master     #371   +/-   ##
=======================================
  Coverage   89.95%   89.95%           
=======================================
  Files           9        9           
  Lines        1155     1155           
=======================================
  Hits         1039     1039           
  Misses        116      116           
Impacted Files Coverage Δ
src/ArrayInterface.jl 85.71% <93.75%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@timholy
Copy link
Member

timholy commented Jan 24, 2023

Nice! Any measurable benefit for load times when you load both packages?

@chriselrod
Copy link
Collaborator Author

Nice! Any measurable benefit for load times when you load both packages?

No measurable difference.
Master:

julia> @time using OrdinaryDiffEq
 17.700264 seconds (72.74 M allocations: 11.539 GiB, 6.41% gc time, 0.53% compilation time)

julia> using PkgCacheInspector

julia> cf_odeq = info_cachefile("OrdinaryDiffEq")
┌ Warning: Replacing module `OrdinaryDiffEq`
└ @ Base loading.jl:1610
Contents of /home/chriselrod/.julia/compiled/v1.10/OrdinaryDiffEq/DlSvy_scZsf.so:
  modules: Any[OrdinaryDiffEq]
  2870 external methods
  17673 new specializations of external methods (Base 52.6%, UnPack 15.2%, Base.Broadcast 14.5%, ...)
  845 external methods with new roots
  29090 external targets
  29749 edges
  file size:   145163320 (138.439 MiB)
  Segment sizes (bytes):
  system:      27658744 ( 23.80%)
  isbits:      85301355 ( 73.40%)
  symbols:       134563 (  0.12%)
  tags:          388409 (  0.33%)
  relocations:  2651694 (  2.28%)
  gvars:          22720 (  0.02%)
  fptrs:          62000 (  0.05%)

PR:

julia> @time using OrdinaryDiffEq
 17.607371 seconds (72.39 M allocations: 11.520 GiB, 6.29% gc time, 0.55% compilation time)

julia> using PkgCacheInspector

julia> cf_odeq = info_cachefile("OrdinaryDiffEq")
┌ Warning: Replacing module `OrdinaryDiffEq`
└ @ Base loading.jl:1610
Contents of /home/chriselrod/.julia/compiled/v1.10/OrdinaryDiffEq/DlSvy_gq1qu.so:
  modules: Any[OrdinaryDiffEq]
  2855 external methods
  17075 new specializations of external methods (Base 52.5%, UnPack 15.7%, Base.Broadcast 13.9%, ...)
  850 external methods with new roots
  28463 external targets
  29296 edges
  file size:   139104280 (132.660 MiB)
  Segment sizes (bytes):
  system:      27335192 ( 24.78%)
  isbits:      79792035 ( 72.32%)
  symbols:       135676 (  0.12%)
  tags:          383162 (  0.35%)
  relocations:  2597879 (  2.35%)
  gvars:          22712 (  0.02%)
  fptrs:          62000 (  0.06%)

But these methods are so cheap to compile, I wouldn't expect a benefit.

@timholy
Copy link
Member

timholy commented Jan 25, 2023

You probably also want to do this analysis on independent rather than dependent packages.

@ChrisRackauckas
Copy link
Member

These will at least show up a lot.

@ChrisRackauckas ChrisRackauckas merged commit 1c78e7c into master Jan 25, 2023
@ChrisRackauckas ChrisRackauckas deleted the snoopprecompile branch January 25, 2023 10:43
@chriselrod
Copy link
Collaborator Author

You probably also want to do this analysis on independent rather than dependent packages.

Yes, as I said on slack, I was expecting this intersection to be empty because the packages are dependent.

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.

3 participants