HDF5.jl icon indicating copy to clipboard operation
HDF5.jl copied to clipboard

Use -O1 optimization level

Open jmert opened this issue 5 years ago • 11 comments

Extracted from #681 so that this can have some more discussion and not slow down that PR. From the other PR:

[This] commit enables a lower optimization level for the entire module in Julia 1.5+. The motivation here is that a lot of functions do not type infer, so we might as well tell the compiler to not try too hard.

jmert avatar Sep 21 '20 15:09 jmert

I think Plots.jl is having to do something similar to this.

musm avatar Sep 21 '20 17:09 musm

Just out of curiosity: rebased on master (307e7dbe8367f573782dfc14c961de7cb64f7849), and running my timing script:

   master precompile:  2.272 ± 0.0294
   master pkg load:    0.624 ± 0.0097
   master pkg test:   47.815 ± 0.6639
optlevel1 precompile:  2.235 ± 0.0462
optlevel1 pkg load:    0.549 ± 0.0110
optlevel1 pkg test:   40.586 ± 0.4779

jmert avatar Oct 29 '20 16:10 jmert

I'm fine with merging this as it does help, as long as we add some comments on it that it's likely unnecessary if we manually fix invalidations in the future.

musm avatar Oct 29 '20 16:10 musm

I'm not in a rush to merge this — was just rebasing some branches and thought I'd just leave a contextual breadcrumb.

Invalidations aren't the thing that this is trying to solve — the invalidations are the same on both master and this branch:

julia> using SnoopCompileCore

julia> invs = @snoopr include("test/runtests.jl")
[ Info: Precompiling HDF5 [f67ccb44-e63f-5c2f-98bd-6dc0ccc4ba2f]
HDF5 version 1.10.4
...

julia> using SnoopCompile

julia> invalidation_trees(invs)
2-element Vector{SnoopCompile.MethodInvalidations}:
 inserting datatype(::Type{foo_hdf5}) in Main at /home/justin/.julia/dev/HDF5/test/compound.jl:32 invalidated:
   mt_backedges: 1: signature Tuple{typeof(datatype), Type} triggered MethodInstance for d_create_external(::HDF5.File, ::String, ::GenericString, ::Type, ::Tuple{Int64, Int64}, ::Int64) (1 children)

 inserting names(x::Union{HDF5.Attributes, HDF5.File, HDF5.Group}) in HDF5 at deprecated.jl:70 invalidated:
   mt_backedges: 1: signature Tuple{typeof(names), Any} triggered MethodInstance for iterate(::Base.Generator{Vector{Any}, typeof(names)}, ::Int64) (3 children)
                 2: signature Tuple{typeof(names), Any} triggered MethodInstance for iterate(::Base.Generator{Vector{Any}, typeof(names)}) (4 children)
   10 mt_cache

It's just that compiler optimizations don't achieve much on poorly type-inferred results, so we can head that off by just telling the compiler to not try very hard at optimizing the generated code.

jmert avatar Oct 29 '20 17:10 jmert

Oh I see, thanks for the clarification. So we have a lot of poorly type-inferred results. It might be tricky to improve the situation on that front.

musm avatar Oct 29 '20 17:10 musm

Yeah, its the very dynamic nature of operations like e.g. read(::Dataset) -> ??? that is going to be the limiting case.

I'm guessing something that will also help is using the other SnoopCompile features and seeing if/where some @nospecialize, ::Any type assertions, and/or other purposeful type-widening might just head off unnecessary inference and compiled specializations.

jmert avatar Oct 29 '20 17:10 jmert

For me timings have now improved, the tests timing difference is about ~ 1 s. And the differences in load and precompile are within 1/10 of a second.

musm avatar Dec 07 '20 21:12 musm

This is what I get on master, a rebased version of this PR, and then a branch where I've additionally removed the deprecations file.

   master precompile:  2.378 ± 0.0274
   master pkg load:    0.562 ± 0.0123
   master pkg test:   52.051 ± 1.6150
optlevel1 precompile:  2.284 ± 0.0135
optlevel1 pkg load:    0.481 ± 0.0120
optlevel1 pkg test:   43.644 ± 0.7541
  no_deps precompile:  2.137 ± 0.0149
  no_deps pkg load:    0.482 ± 0.0092
  no_deps pkg test:   43.443 ± 0.5663

So package precompile and load times are very close now, but optlevel 1 still reduces the entire test suite time by ~8 seconds (~15% reduction from master).

jmert avatar Dec 09 '20 20:12 jmert

I'd also recommend testing/benchmarking

if isdefined(Base, :Experimental) && isdefined(Base.Experimental, Symbol("@max_methods"))
    @eval Base.Experimental.@max_methods 1
    @eval Base.Experimental.@optlevel 0
end

musm avatar May 31 '22 21:05 musm

Last CI run on master, julia-actions/julia-runtest@latest took 1m 58s for ubuntu-latest Julia 1. On this pull request, julia-actions/julia-runtest@latest took 1m 46s for ubuntu-latest Julia 1.

mkitti avatar May 31 '22 21:05 mkitti

Looking further, it's a bit of a wash.

Platform master -O1 optimization
ubuntu-latest 1m 58 s 1m 46 s
macOS 1m 55s 2m 16s
windows x64 2m 15s 2m 2s
windows x86 2m 33s 2m 43 s

mkitti avatar May 31 '22 21:05 mkitti