-
Notifications
You must be signed in to change notification settings - Fork 93
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
Use sorted OrderedSet in FieldHandler #654
Conversation
This changes `FieldHandler.cellset` to be a sorted `OrderedSet` instead of a `Set`. This ensures that loops over sub-domains are done in ascending cell order. Since e.g. cells, node coordinates, and dofs are stored in ascending cell order this gives a significant performance boost to loops over sub-domains, i.e. assembly-style loops. In particular, this removes the performance gap between `MixedDofHandler` and `DofHandler` in the `create_sparsity_pattern` benchmark in #629. This is a minimal/initial step towards #625 that can be done before the `DofHandler` merge and rework of `FieldHandler`/`SubDofHandler`.
Perhaps |
This patch creates a `BitSet` of `FieldHandler.cellset` in loops, in particular in `close!(::DofHandler)` and `create_sparsity_pattern(::DofHandler)`. Since `BitSet`s are sorted this ensures that these loops are done in ascending cell order, which gives a performance boost due to better memory locality. This is an even smaller change than #654 (and #625) which should be completely non-breaking since the type of `FieldHandler.cellset` is not changed. Larger refactoring, such as using `BitSet` or `OrderedSet` will be done in the `FieldHandler/`SubDofHandler` rework.
This patch creates a `BitSet` of `FieldHandler.cellset` in loops, in particular in `close!(::DofHandler)` and `create_sparsity_pattern(::DofHandler)`. Since `BitSet`s are sorted this ensures that these loops are done in ascending cell order, which gives a performance boost due to better memory locality. This is an even smaller change than #654 (and #625) which should be completely non-breaking since the type of `FieldHandler.cellset` is not changed. Larger refactoring, such as using `BitSet` or `OrderedSet` will be done in the `FieldHandler/`SubDofHandler` rework.
Realized today that at least for threading, the ordered set must be collected: https://julialang.slack.com/archives/C6SMTHQ3T/p1683401329225109. using OrderedCollections
vec = [1,3,5];
v = zeros(Int, 5);
oset = OrderedSet(vec);
Threads.@threads :static for i in vec
v[i] = Threads.threadid()
end
Threads.@threads :static for i in oset
v[i] = Threads.threadid()
end ERROR: TaskFailedException
Stacktrace:
[1] wait
@ .\task.jl:345 [inlined]
[2] threading_run(fun::var"#45#threadsfor_fun#5"{var"#45#threadsfor_fun#4#6"{OrderedSet{Int64}}}, static::Bool)
@ Base.Threads .\threadingconstructs.jl:38
[3] top-level scope
@ .\threadingconstructs.jl:93
nested task error: MethodError: no method matching firstindex(::OrderedSet{Int64})
Closest candidates are:
firstindex(::Any, ::Any) at abstractarray.jl:402
firstindex(::Tuple) at tuple.jl:25
firstindex(::Pair) at pair.jl:50
...
Stacktrace:
[1] #45#threadsfor_fun#4
@ .\threadingconstructs.jl:69 [inlined]
[2] #45#threadsfor_fun
@ .\threadingconstructs.jl:51 [inlined]
[3] (::Base.Threads.var"#1#2"{var"#45#threadsfor_fun#5"{var"#45#threadsfor_fun#4#6"{OrderedSet{Int64}}}, Int64})()
@ Base.Threads .\threadingconstructs.jl:30 |
This patch changes the use of `Set` to `OrderedSet` in the grid. This means that e.g. loops over these sets follow the original specified order. This is important for performance since it can reduce cache-misses, for example. Fixes #631. Closes #654. Co-authored-by: Fredrik Ekre <[email protected]> Co-authored-by: Dennis Ogiermann <[email protected]>
This patch changes the use of `Set` to `OrderedSet` in the grid. This means that e.g. loops over these sets follow the original specified order. This is important for performance since it can reduce cache-misses, for example. Fixes #631. Closes #654. Co-authored-by: Fredrik Ekre <[email protected]> Co-authored-by: Dennis Ogiermann <[email protected]>
This patch changes the use of `Set` to `OrderedSet` in the grid. This means that e.g. loops over these sets follow the original specified order. This is important for performance since it can reduce cache-misses, for example. Fixes #631. Closes #654. Co-authored-by: Fredrik Ekre <[email protected]> Co-authored-by: Dennis Ogiermann <[email protected]>
This patch changes the use of `Set` to `OrderedSet` in the grid. This means that e.g. loops over these sets follow the original specified order. This is important for performance since it can reduce cache-misses, for example. Fixes #631. Closes #654. Co-authored-by: Fredrik Ekre <[email protected]> Co-authored-by: Dennis Ogiermann <[email protected]>
This patch changes the use of `Set` to `OrderedSet` in the grid. This means that e.g. loops over these sets follow the original specified order. This is important for performance since it can reduce cache-misses, for example. Fixes #631. Closes #654. Co-authored-by: Fredrik Ekre <[email protected]> Co-authored-by: Dennis Ogiermann <[email protected]>
This changes
FieldHandler.cellset
to be a sortedOrderedSet
instead of aSet
. This ensures that loops over sub-domains are done in ascending cell order.Since e.g. cells, node coordinates, and dofs are stored in ascending cell order this gives a significant performance boost to loops over sub-domains, i.e. assembly-style loops. In particular, this removes the performance gap between
MixedDofHandler
andDofHandler
in thecreate_sparsity_pattern
benchmark in #629.This is a minimal/initial step towards #625 that can be done before the
DofHandler
merge and rework ofFieldHandler
/SubDofHandler
.