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

Adding sync_wait_with_variant #5956

Closed
wants to merge 6 commits into from

Conversation

hcq9102
Copy link
Contributor

@hcq9102 hcq9102 commented Jul 17, 2022

Any background context you want to provide?

P2300R5: sync_wait_with_variant
sync_wait_with_variant, which accepts any sender, but returns an optional whose value type is the variant of all the possible tuples sent by the input sender:

Checklist

  • I have added a new feature and have added tests to go along with it.

@hcq9102 hcq9102 marked this pull request as ready for review July 17, 2022 14:09
@hkaiser hkaiser added type: enhancement type: compatibility issue category: senders/receivers Implementations of the p0443r14 / p2300 + p1897 proposals labels Jul 17, 2022
@hkaiser hkaiser added this to the 1.9.0 milestone Jul 17, 2022
Copy link
Member

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

Very nice! Please add the new file here: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/execution/CMakeLists.txt#L26, and add the new test to the build system (here: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/execution/tests/unit/CMakeLists.txt#L19).

Also, now that you have a working version that duplicates 95% of the sync_wait code, would you see a way to combine the implementations of sync_wait and sync_wait_with_variant to reduce the amount of code repetition?

@hcq9102
Copy link
Contributor Author

hcq9102 commented Jul 17, 2022

Also, now that you have a working version that duplicates 95% of the sync_wait code, would you see a way to combine the implementations of sync_wait and sync_wait_with_variant to reduce the amount of code repetition?

Sure, will do.

@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-07-18T15:18:52+00:00
HPX Commitd5655f832c7927
Datetime2022-05-31T15:13:01.357969+02:002022-07-18T17:31:13.698133+02:00
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostnamenid01193nid01193
Clusternamedaintdaint
Envfile

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch=

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-07-18T15:18:52+00:00
HPX Commitd5655f832c7927
Datetime2022-05-31T15:13:18.026239+02:002022-07-18T17:31:30.343351+02:00
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostnamenid01193nid01193
Clusternamedaintdaint
Envfile

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)-(=)
Stream Benchmark - Scale(=)(=)(=)
Stream Benchmark - Triad(=)(=)(=)
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2021-11-10T19:14:21+00:002022-07-18T15:18:52+00:00
HPX Commit71d8dbe32c7927
Datetime2021-11-10T20:28:18.266961+01:002022-07-18T17:31:45.627798+02:00
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostnamenid00120nid01193
Clusternamedaintdaint
Envfile

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)=(=)

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-07-18T15:33:17+00:00
HPX Commitd5655f844ef1d3
Hostnamenid01193nid00435
Clusternamedaintdaint
Datetime2022-05-31T15:13:01.357969+02:002022-07-18T17:47:08.838396+02:00
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch=

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-07-18T15:33:17+00:00
HPX Commitd5655f844ef1d3
Hostnamenid01193nid00435
Clusternamedaintdaint
Datetime2022-05-31T15:13:18.026239+02:002022-07-18T17:47:25.501653+02:00
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)(=)(=)
Stream Benchmark - Scale(=)(=)(=)
Stream Benchmark - Triad(=)(=)(=)
Stream Benchmark - Copy(=)-(=)

Info

PropertyBeforeAfter
HPX Datetime2021-11-10T19:14:21+00:002022-07-18T15:33:17+00:00
HPX Commit71d8dbe44ef1d3
Hostnamenid00120nid00435
Clusternamedaintdaint
Datetime2021-11-10T20:28:18.266961+01:002022-07-18T17:47:40.865125+02:00
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@hcq9102 hcq9102 force-pushed the sync_wait_variant branch 5 times, most recently from 13b84ab to 722ec9a Compare July 19, 2022 15:25
@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
HPX Commitd5655f8258527e
HPX Datetime2022-05-31T12:57:29+00:002022-07-19T16:50:56+00:00
Datetime2022-05-31T15:13:01.357969+02:002022-07-19T19:11:51.673858+02:00
Envfile
Clusternamedaintdaint
Hostnamenid01193nid01323
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
HPX Commitd5655f8258527e
HPX Datetime2022-05-31T12:57:29+00:002022-07-19T16:50:56+00:00
Datetime2022-05-31T15:13:18.026239+02:002022-07-19T19:12:08.672710+02:00
Envfile
Clusternamedaintdaint
Hostnamenid01193nid01323
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)-(=)
Stream Benchmark - Scale(=)(=)=
Stream Benchmark - Triad(=)-(=)
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
HPX Commit71d8dbe258527e
HPX Datetime2021-11-10T19:14:21+00:002022-07-19T16:50:56+00:00
Datetime2021-11-10T20:28:18.266961+01:002022-07-19T19:12:24.086403+02:00
Envfile
Clusternamedaintdaint
Hostnamenid00120nid01323
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@hcq9102 hcq9102 force-pushed the sync_wait_variant branch 9 times, most recently from 7253a3c to dd3af2d Compare July 22, 2022 15:14
@hcq9102 hcq9102 force-pushed the sync_wait_variant branch 2 times, most recently from 32b50fe to 3ea62d8 Compare July 31, 2022 14:05
@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)=(=)

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-07-31T14:05:57+00:00
HPX Commitd5655f8a866cc4
Hostnamenid01193nid01193
Datetime2022-05-31T15:13:01.357969+02:002022-07-31T16:22:36.824275+02:00
Clusternamedaintdaint
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch(=)

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-07-31T14:05:57+00:00
HPX Commitd5655f8a866cc4
Hostnamenid01193nid01193
Datetime2022-05-31T15:13:18.026239+02:002022-07-31T16:22:53.800696+02:00
Clusternamedaintdaint
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)-(=)
Stream Benchmark - Scale(=)(=)(=)
Stream Benchmark - Triad(=)-(=)
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2021-11-10T19:14:21+00:002022-07-31T14:05:57+00:00
HPX Commit71d8dbea866cc4
Hostnamenid00120nid01193
Datetime2021-11-10T20:28:18.266961+01:002022-07-31T16:23:09.089371+02:00
Clusternamedaintdaint
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@hcq9102 hcq9102 force-pushed the sync_wait_variant branch 6 times, most recently from 2248cad to c7e9ba4 Compare August 5, 2022 05:28
@hcq9102 hcq9102 force-pushed the sync_wait_variant branch 2 times, most recently from 7a3e9c3 to 5663170 Compare August 10, 2022 18:11
@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-08-10T18:11:09+00:00
HPX Commitd5655f8723fce0
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile
Hostnamenid01193nid01159
Datetime2022-05-31T15:13:01.357969+02:002022-08-10T20:26:22.517883+02:00
Clusternamedaintdaint

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-08-10T18:11:09+00:00
HPX Commitd5655f8723fce0
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile
Hostnamenid01193nid01159
Datetime2022-05-31T15:13:18.026239+02:002022-08-10T20:26:39.667478+02:00
Clusternamedaintdaint

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)(=)(=)
Stream Benchmark - Scale(=)(=)(=)
Stream Benchmark - Triad(=)(=)(=)
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2021-11-10T19:14:21+00:002022-08-10T18:11:09+00:00
HPX Commit71d8dbe723fce0
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile
Hostnamenid00120nid01159
Datetime2021-11-10T20:28:18.266961+01:002022-08-10T20:26:54.843070+02:00
Clusternamedaintdaint

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@hcq9102 hcq9102 force-pushed the sync_wait_variant branch 2 times, most recently from 88d09a7 to 210f1c9 Compare August 12, 2022 16:08
@hkaiser
Copy link
Member

hkaiser commented Aug 18, 2022

@hcq9102 things should be fine now. Please note that I also have rebased this branch onto master.

@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-08-29T16:20:27+00:00
HPX Commitd5655f84830586
Datetime2022-05-31T15:13:01.357969+02:002022-08-29T19:15:04.416369+02:00
Hostnamenid01193nid00013
Envfile
Clusternamedaintdaint
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
HPX Datetime2022-05-31T12:57:29+00:002022-08-29T16:20:27+00:00
HPX Commitd5655f84830586
Datetime2022-05-31T15:13:18.026239+02:002022-08-29T19:15:21.732327+02:00
Hostnamenid01193nid00013
Envfile
Clusternamedaintdaint
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)(=)(=)
Stream Benchmark - Scale(=)(=)(=)
Stream Benchmark - Triad(=)(=)(=)
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2021-11-10T19:14:21+00:002022-08-29T16:20:27+00:00
HPX Commit71d8dbe4830586
Datetime2021-11-10T20:28:18.266961+01:002022-08-29T19:15:37.032623+02:00
Hostnamenid00120nid00013
Envfile
Clusternamedaintdaint
Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@hkaiser
Copy link
Member

hkaiser commented Oct 5, 2022

This can be closed as it is subsumed by #6015

@hkaiser hkaiser closed this Oct 5, 2022
@hcq9102
Copy link
Contributor Author

hcq9102 commented Oct 5, 2022

#6015 got merged. We don't need this one anymore.(sync_wait_with_variant integrating with sync_wait)

@gonidelis gonidelis removed this from the 1.9.0 milestone Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: senders/receivers Implementations of the p0443r14 / p2300 + p1897 proposals type: compatibility issue type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants