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

Use TeamPolicy in Details::expandHalfToFull to improve performance #811

Merged
merged 2 commits into from
Jan 5, 2023

Conversation

dalg24
Copy link
Contributor

@dalg24 dalg24 commented Jan 5, 2023

In #809 we had opted to merge in the version with RangePolicy even though it was commented out with a macro guard in the code I was using to measure performance.
I don't have numbers handy to share but with the TeamPolicy version, findHalfNeighborList+expandHalfToFull is 15% faster than the "classic" BVH query implementation, with a RangePolicy it is slower than the current status quo.

@dalg24 dalg24 added the performance Something is slower than it should be label Jan 5, 2023
Copy link
Contributor

@aprokop aprokop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch itself looks fine. I think another minor improvement that should be done is doing
auto counts = KokkosExt::clone(space, offsets);
and then doing
indices(Kokkos::atomic_fetch_inc(&counts(i)) = <blah>;
inside the parallel_for.

This would

  • avoid fetching offsets storage
  • avoid doing index summation

@dalg24
Copy link
Contributor Author

dalg24 commented Jan 5, 2023

Current PR

|   |-> 6.61e-02 sec 8.1% 99.3% 0.0% 0.7% 1.06e+02 1 ArborX::Experimental::HalfToFull [region]
|   |   |-> 4.40e-02 sec 5.4% 100.0% 0.0% ------ 1 ArborX::Experimental::HalfToFull::rewrite [for]
|   |   |-> 2.15e-02 sec 2.6% 100.0% 0.0% ------ 1 ArborX::Experimental::HalfToFull::count [for]

VS your suggestion

|   |-> 6.53e-02 sec 8.0% 99.5% 0.0% 0.5% 1.07e+02 1 ArborX::Experimental::HalfToFull [region]
|   |   |-> 4.33e-02 sec 5.3% 100.0% 0.0% ------ 1 ArborX::Experimental::HalfToFull::rewrite [for]
|   |   |-> 2.15e-02 sec 2.6% 100.0% 0.0% ------ 1 ArborX::Experimental::HalfToFull::count [for]

Co-Authored-By: Andrey Prokopenko <prokopenkoav@ornl.gov>
@aprokop aprokop merged commit 0e96374 into arborx:master Jan 5, 2023
@dalg24 dalg24 deleted the team_policy_expand_half_to_full branch January 5, 2023 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Something is slower than it should be
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants