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

Improve performance by replacing abstract Integer with concrete Int in arrays and dicts #50

Closed
benzwick opened this issue Aug 31, 2019 · 3 comments · Fixed by #58
Closed

Comments

@benzwick
Copy link
Collaborator

We might be able to improve performance by using concrete Int types instead of abstract Integer types in the following places:

test/test_parse_mesh.jl
30:    model["elements"] = Dict{Integer, Vector{Integer}}()
32:    model["element_types"] = Dict{Integer, Symbol}()

README.md
18:  "element_types" => Dict{Integer,Symbol}(Pair{Integer,Symbol}(2, :Tet4),Pair{I…
19:  "elements"      => Dict{Integer,Array{Integer,1}}(Pair{Integer,Array{Integer,…

src/create_surface_elements.jl
62:    result = Tuple{Symbol, Vector{Integer}}[]

src/parse_model.jl
18:    node_sets :: Dict{String, Vector{Integer}}
21:    element_sets :: Dict{String, Vector{Integer}}

src/parse_mesh.jl
97:    ids = Integer[]
144:    ids = Integer[]
180:    data = Integer[]
231:    indexes = Integer[]
249:    model["elements"] = Dict{Integer, Vector{Integer}}()
250:    model["element_types"] = Dict{Integer, Symbol}()

Some quick benchmarks:

julia> using BenchmarkTools

julia> a = Integer[]
0-element Array{Integer,1}

julia> @benchmark for i = 1:1000 push!(a, i^2+1) end
BenchmarkTools.Trial: 
  memory estimate:  30.56 KiB
  allocs estimate:  1956
  --------------
  minimum time:     22.861 μs (0.00% GC)
  median time:      27.172 μs (0.00% GC)
  mean time:        55.590 μs (0.00% GC)
  maximum time:     252.070 ms (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     1
julia> using BenchmarkTools

julia> b = Int[]
0-element Array{Int64,1}

julia> @benchmark for i = 1:1000 push!(b, i^2+1) end
BenchmarkTools.Trial: 
  memory estimate:  15.28 KiB
  allocs estimate:  978
  --------------
  minimum time:     17.985 μs (0.00% GC)
  median time:      22.930 μs (0.00% GC)
  mean time:        25.419 μs (0.00% GC)
  maximum time:     313.297 μs (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     1

The maximum time is probably most important because we usually only do this once for each array.

See Avoid containers with abstract type parameters

@TeroFrondelius
Copy link
Member

If you are willing to do a pull request, I will merge it. Currently, we are lacking an active maintenance of this package, thus after a couple of pull request I can also give contributor rights.

@benzwick
Copy link
Collaborator Author

OK, when I have time I will look into this in more detail. We could also use e.g. UInt32 for storing indices to save space. This would allow node numbers up to

2^32 - 1 = 4294967295

which should be enough for most models that use .inp as input.

See here for available number types: Integers and Floating-Point Numbers

@TeroFrondelius
Copy link
Member

Also a good idea. However, use of UInt32 needs to be optional (can be default), because we use stupidly high node numbers in Abaqus input files. We have standardized number ranges for different components to avoid node number collision. Our connections nodes have the highest numbers.

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 a pull request may close this issue.

2 participants