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

Add require_started sender adaptor #869

Merged
merged 4 commits into from
Dec 4, 2023
Merged

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented Nov 24, 2023

Fixes #790.

Comments on the name are especially welcome. I found require_started quite descriptive as it signals that the user is required to make sure that the sender is started. It also mirrors ensure_started which actually connects and starts a sender.

The require_started sender has an empty state to make it somewhat sensible to detect unstarted senders. E.g. just() is copyable, but it's also possible to connect it after moving it. This is not allowed by the require_started sender. The require_started sender stores the wrapped sender internally in an optional, and when moving a require_started sender the optional is emptied.

A require_started sender can be allowed to not be started, but it has to be explicitly asked for with discard.

When a require_started sender is not connected, or when it's connected but the operation state not started, the destructor will terminate (by default). Assigning to unstarted senders also leads to termination.

Mainly for testing purposes the behaviour on unstarted senders is customizable. The default behaviour is to terminate, since that reports the error immediately and cannot be ignored. The behaviour can be changed to throw instead with a flag passed to the sender constructor, or changed afterwards with set_mode. I don't expect this second mode to be used much in practice, but it's useful to have as an alternative at least for testing. The choice of mode is disabled with stdexec, since stdexec requires nothrow destructible senders. In this case terminate is always used.

@msimberg msimberg added this to the 0.21.0 milestone Nov 24, 2023
@msimberg msimberg self-assigned this Nov 24, 2023
@msimberg msimberg marked this pull request as draft November 24, 2023 16:29
@msimberg
Copy link
Contributor Author

cscs-ci run

@pika-bot
Copy link
Collaborator

Performance test report

pika Performance

Comparison

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

Info

PropertyBeforeAfter
pika Datetime2023-08-21T11:44:55+00:002023-11-24T16:29:13+00:00
pika Commit02f9de8562edc0
Hostnamenid01181nid00459
Clusternamedaintdaint
Envfile
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime2023-08-21T13:50:51.685166+02:002023-11-24T17:41:58.929351+01:00

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 (>10%)
++/--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…

@msimberg msimberg force-pushed the require-started branch 3 times, most recently from eb85c98 to 77ae5de Compare November 27, 2023 10:47
@pika-bot
Copy link
Collaborator

Performance test report

pika Performance

Comparison

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

Info

PropertyBeforeAfter
pika Datetime2023-08-21T11:44:55+00:002023-11-27T10:48:01+00:00
pika Commit02f9de82d2f6d7
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Clusternamedaintdaint
Envfile
Hostnamenid01181nid01537
Datetime2023-08-21T13:50:51.685166+02:002023-11-27T11:57:35.682158+01:00

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 (>10%)
++/--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…

@msimberg
Copy link
Contributor Author

cscs-ci run

@msimberg msimberg marked this pull request as ready for review November 27, 2023 12:30
@msimberg
Copy link
Contributor Author

cscs-ci run

@msimberg msimberg enabled auto-merge November 29, 2023 09:41
@msimberg
Copy link
Contributor Author

cscs-ci run

1 similar comment
@msimberg
Copy link
Contributor Author

msimberg commented Dec 4, 2023

cscs-ci run

@msimberg
Copy link
Contributor Author

msimberg commented Dec 4, 2023

cscs-ci run

1 similar comment
@msimberg
Copy link
Contributor Author

msimberg commented Dec 4, 2023

cscs-ci run

@msimberg msimberg added this pull request to the merge queue Dec 4, 2023
@msimberg
Copy link
Contributor Author

msimberg commented Dec 4, 2023

cscs-ci run

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2023
@msimberg msimberg enabled auto-merge December 4, 2023 12:07
@msimberg
Copy link
Contributor Author

msimberg commented Dec 4, 2023

cscs-ci run

@pika-bot
Copy link
Collaborator

pika-bot commented Dec 4, 2023

Performance test report

pika Performance

Comparison

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

Info

PropertyBeforeAfter
pika Datetime2023-08-21T11:44:55+00:002023-12-04T12:47:56+00:00
pika Commit02f9de8a5d9162
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime2023-08-21T13:50:51.685166+02:002023-12-04T13:56:54.067011+01:00
Clusternamedaintdaint
Hostnamenid01181nid01181
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 (>10%)
++/--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…

@msimberg
Copy link
Contributor Author

msimberg commented Dec 4, 2023

cscs-ci run

@msimberg msimberg added this pull request to the merge queue Dec 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2023
@msimberg msimberg added this pull request to the merge queue Dec 4, 2023
Merged via the queue into pika-org:main with commit 5664646 Dec 4, 2023
65 of 67 checks passed
@msimberg msimberg deleted the require-started branch December 4, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Archive
Development

Successfully merging this pull request may close these issues.

Add debug sender adaptor to diagnose unstarted senders
2 participants