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.

Sign up for GitHub

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

Jump to bottom

Switch knn to the same stack approach as radius Karras #308

Merged
merged 2 commits into from
May 26, 2020
Merged

Switch knn to the same stack approach as radius Karras #308

merged 2 commits into from
May 26, 2020

Conversation

aprokop
Copy link
Contributor

@aprokop aprokop commented May 20, 2020

No description provided.

@aprokop aprokop added the performance Something is slower than it should be label May 20, 2020
@aprokop
Copy link
Contributor Author

aprokop commented May 20, 2020 •
edited
Loading

These results are copied from this comment:

BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/1/0/2/manual_time_median                    -0.0663         -0.0666         54327         50722         54322         50702
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/1/0/2/manual_time_median                  -0.0612         -0.0610        566880        532189        566682        532129
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/1/0/2/manual_time_median                -0.0596         -0.0596       6029418       5669810       6028907       5669315
BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/1/1/3/manual_time_median                    -0.0710         -0.0710         53969         50137         53966         50134
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/1/1/3/manual_time_median                  -0.0772         -0.0772        730431        674011        730349        673932
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/1/1/3/manual_time_median                -0.0795         -0.0795      10180027       9370534      10179119       9369848
BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/0/0/2/manual_time_median                    -0.0640         -0.0640         55763         52193         55758         52189
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/0/0/2/manual_time_median                  -0.0660         -0.0659        599138        559618        599067        559561
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/0/0/2/manual_time_median                -0.0639         -0.0639       6740405       6309719       6738247       6307696
BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/0/1/3/manual_time_median                    -0.0680         -0.0680         60932         56788         60927         56783
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/0/1/3/manual_time_median                  -0.0727         -0.0727        861348        798730        861240        798631
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/0/1/3/manual_time_median                -0.0723         -0.0724      12404885      11507511      12401211      11503907
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/1/0/2/manual_time_median                    +0.0004         +0.0002          1812          1813          1814          1815
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/1/0/2/manual_time_median                  -0.0398         -0.0388         15469         14853         15086         14500
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/1/0/2/manual_time_median                -0.0424         -0.0421        158063        151365        156455        149876
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/1/1/3/manual_time_median                    -0.0179         -0.0179          2226          2187          2228          2189
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/1/1/3/manual_time_median                  -0.0562         -0.0560         26993         25477         25014         23612
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/1/1/3/manual_time_median                -0.0606         -0.0611        403118        378700        341701        320819
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/0/0/2/manual_time_median                    -0.0136         -0.0138          1567          1546          1569          1548
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/0/0/2/manual_time_median                  -0.0470         -0.0469         15103         14392         14421         13745
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/0/0/2/manual_time_median                -0.0424         -0.0417        202123        193559        191096        183121
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/0/1/3/manual_time_median                    -0.0145         -0.0145          1699          1675          1702          1677
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/0/1/3/manual_time_median                  -0.0465         -0.0466         21312         20321         21306         20312
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/0/1/3/manual_time_median                -0.0445         -0.0447        331383        316648        329184        314462
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/1/0/2/manual_time_median                      -0.0174         -0.0169          1650          1621          1743          1714
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/1/0/2/manual_time_median                    -0.2539         -0.2433         10055          7502         10518          7959
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/1/0/2/manual_time_median                  -0.3171         -0.3131         77497         52925         78361         53824
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/1/1/3/manual_time_median                      -0.0186         -0.0180          1690          1658          1781          1749
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/1/1/3/manual_time_median                    -0.2251         -0.2212         12451          9648         12863         10017
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/1/1/3/manual_time_median                  -0.2394         -0.2382        141450        107582        142337        108428
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/0/0/2/manual_time_median                      -0.0111         -0.0092          1147          1134          1238          1227
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/0/0/2/manual_time_median                    -0.3659         -0.3473          9203          5836          9632          6287
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/0/0/2/manual_time_median                  -0.3203         -0.3173        103264         70191        104132         71095
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/0/1/3/manual_time_median                      -0.0281         -0.0260          1213          1179          1303          1269
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/0/1/3/manual_time_median                    -0.3960         -0.3861         18614         11242         19078         11713
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/0/1/3/manual_time_median                  -0.4495         -0.4485        309539        170402        310414        171200

@aprokop
Copy link
Contributor Author

aprokop commented May 24, 2020

I cleaned up the branch, making use of Kokkos::pair (not sure why I had trouble before). I also retested it:

BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/1/0/2/manual_time_median                    -0.0433         -0.0433         55462         53060         55457         53054
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/1/0/2/manual_time_median                  -0.0422         -0.0422        578920        554514        578859        554455
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/1/0/2/manual_time_median                -0.0419         -0.0419       6156422       5898332       6155885       5897802
BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/1/1/3/manual_time_median                    -0.0495         -0.0495         55400         52656         55395         52654
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/1/1/3/manual_time_median                  -0.0547         -0.0547        750784        709715        750704        709642
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/1/1/3/manual_time_median                -0.0565         -0.0565      10430032       9840629      10429098       9839747
BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/0/0/2/manual_time_median                    -0.0454         -0.0454         56413         53853         56409         53849
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/0/0/2/manual_time_median                  -0.0453         -0.0453        606094        578654        606019        578580
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/0/0/2/manual_time_median                -0.0448         -0.0449       6813214       6507695       6811214       6505692
BM_knn_search<ArborX::BVH<Serial>>/10000/10000/10/0/1/3/manual_time_median                    -0.0572         -0.0570         61574         58051         61557         58047
BM_knn_search<ArborX::BVH<Serial>>/100000/100000/10/0/1/3/manual_time_median                  -0.0609         -0.0609        865322        812608        865209        812505
BM_knn_search<ArborX::BVH<Serial>>/1000000/1000000/10/0/1/3/manual_time_median                -0.0617         -0.0618      12419649      11652939      12416304      11649564
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/1/0/2/manual_time_median                    -0.0328         -0.0327          1936          1873          1938          1875
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/1/0/2/manual_time_median                  -0.0432         -0.0438         16030         15338         15613         14929
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/1/0/2/manual_time_median                -0.0419         -0.0431        162552        155736        160857        153925
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/1/1/3/manual_time_median                    -0.0469         -0.0469          2367          2256          2369          2258
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/1/1/3/manual_time_median                  -0.0641         -0.0641         27943         26151         26057         24387
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/1/1/3/manual_time_median                -0.0750         -0.0652        417020        385737        352754        329767
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/0/0/2/manual_time_median                    -0.0392         -0.0391          1661          1596          1663          1598
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/0/0/2/manual_time_median                  -0.0479         -0.0502         15584         14837         14855         14109
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/0/0/2/manual_time_median                -0.0423         -0.0432        204764        196095        194798        186376
BM_knn_search<ArborX::BVH<OpenMP>>/10000/10000/10/0/1/3/manual_time_median                    -0.0461         -0.0461          1790          1708          1792          1710
BM_knn_search<ArborX::BVH<OpenMP>>/100000/100000/10/0/1/3/manual_time_median                  -0.0556         -0.0555         21658         20454         21657         20454
BM_knn_search<ArborX::BVH<OpenMP>>/1000000/1000000/10/0/1/3/manual_time_median                -0.0532         -0.0532        332454        314763        331067        313448
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/1/0/2/manual_time_median                      -0.0208         -0.0200          1637          1603          1730          1695
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/1/0/2/manual_time_median                    -0.2749         -0.2409         10379          7526         10518          7984
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/1/0/2/manual_time_median                  -0.3936         -0.3882         76953         46668         77697         47537
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/1/1/3/manual_time_median                      -0.0236         -0.0224          1688          1648          1780          1740
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/1/1/3/manual_time_median                    -0.2269         -0.2214         12300          9509         12765          9938
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/1/1/3/manual_time_median                  -0.3339         -0.3316        140414         93536        141183         94370
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/0/0/2/manual_time_median                      -0.0177         -0.0158          1152          1131          1244          1224
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/0/0/2/manual_time_median                    -0.3626         -0.3450          9162          5840          9630          6308
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/0/0/2/manual_time_median                  -0.3173         -0.3156        102620         70058        103518         70850
BM_knn_search<ArborX::BVH<Cuda>>/10000/10000/10/0/1/3/manual_time_median                      -0.0308         -0.0286          1213          1175          1304          1267
BM_knn_search<ArborX::BVH<Cuda>>/100000/100000/10/0/1/3/manual_time_median                    -0.3845         -0.3741         18518         11397         18954         11863
BM_knn_search<ArborX::BVH<Cuda>>/1000000/1000000/10/0/1/3/manual_time_median                  -0.4534         -0.4526        304231        166293        305107        167004

This is ready for merge.

@aprokop
Copy link
Contributor Author

aprokop commented May 24, 2020

@dalg24 The only possible cleanup thing could be using std::tie when popping from the stack. But it seems tricky, and I don't think it's worth it.

@aprokop aprokop merged commit 58d0652 into arborx:master May 26, 2020
@aprokop aprokop deleted the knn_stack_refactor branch May 26, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dalg24 dalg24 dalg24 approved these changes

Assignees
No one assigned
Labels
performance Something is slower than it should be
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aprokop @dalg24