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

Add const reference handling test for let_value #472

Merged
merged 3 commits into from
Oct 10, 2022
Merged

Add const reference handling test for let_value #472

merged 3 commits into from
Oct 10, 2022

Conversation

aurianer
Copy link
Contributor

@aurianer aurianer commented Oct 5, 2022 •
edited
Loading

Add reference handling test for let_value sender.
Fix partially #284

@aurianer aurianer added this to the 0.10.0 milestone Oct 5, 2022
@aurianer aurianer self-assigned this Oct 5, 2022
@aurianer aurianer changed the title Ref handling let value Add const reference handling test for let_value Oct 5, 2022
@aurianer aurianer mentioned this pull request Oct 5, 2022
Closed
12 tasks
@aurianer aurianer force-pushed the ref_handling_let_value branch from c88ce91 to 46e53ba Compare October 6, 2022 09:21
@pika-bot
Copy link
Collaborator

pika-bot commented Oct 6, 2022

Performance test report

pika Performance

Comparison

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

Info

PropertyBeforeAfter
pika Commitd45937dcf7b2a8
pika Datetime2022-05-17T10:12:00+00:002022-10-06T09:21:47+00:00
Hostnamenid01398nid00993
Datetime2022-05-17T12:20:04.953138+02:002022-10-06T11:32:46.443753+02:00
Clusternamedaintdaint
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
Envfile

Comparison

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

Info

PropertyBeforeAfter
pika Commit190f189cf7b2a8
pika Datetime2022-09-16T08:18:06+00:002022-10-06T09:21:47+00:00
Hostnamenid00074nid00993
Datetime2022-09-16T10:25:01.976661+02:002022-10-06T11:33:01.113111+02:00
Clusternamedaintdaint
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
Envfile

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
Stream Benchmark - Add(=)-++
Stream Benchmark - Scale+++(=)++
Stream Benchmark - Triad+++++
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
pika Commitd45937dcf7b2a8
pika Datetime2022-05-17T10:12:00+00:002022-10-06T09:21:47+00:00
Hostnamenid01398nid00993
Datetime2022-05-17T12:20:34.006375+02:002022-10-06T11:33:15.963979+02:00
Clusternamedaintdaint
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
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
msimberg requested changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@msimberg msimberg left a comment

Choose a reason for hiding this comment

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

Looks good to me. Can you also add a test with a reference error type? It should just work without any changes to the implementation.

@aurianer
Copy link
Contributor Author

aurianer commented Oct 6, 2022

Yes will do :)

@aurianer aurianer force-pushed the ref_handling_let_value branch from 46e53ba to d291421 Compare October 10, 2022 13:43
msimberg
msimberg approved these changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@msimberg msimberg left a comment

Choose a reason for hiding this comment

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

Thanks!

@msimberg
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request Oct 10, 2022
@bors @aurianer
Merge #472
3c71042 
472: Add const reference handling test for `let_value` r=msimberg a=aurianer

Add reference handling test for `let_value` sender.
Fix partially #284

Co-authored-by: Auriane R <aurianer@cscs.ch>
Co-authored-by: aurianer <aurianer@cscs.ch>
@bors
Copy link
Contributor

bors bot commented Oct 10, 2022

Build failed:

@msimberg
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request Oct 10, 2022
@bors @aurianer
Merge #472
e7bcc7e 
472: Add const reference handling test for `let_value` r=msimberg a=aurianer

Add reference handling test for `let_value` sender.
Fix partially #284

Co-authored-by: Auriane R <aurianer@cscs.ch>
Co-authored-by: aurianer <aurianer@cscs.ch>
@bors
Copy link
Contributor

bors bot commented Oct 10, 2022

Build failed:

@msimberg
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request Oct 10, 2022
@bors @aurianer
Merge #472
58ed143 
472: Add const reference handling test for `let_value` r=msimberg a=aurianer

Add reference handling test for `let_value` sender.
Fix partially #284

Co-authored-by: Auriane R <aurianer@cscs.ch>
Co-authored-by: aurianer <aurianer@cscs.ch>
@bors
Copy link
Contributor

bors bot commented Oct 10, 2022

Build failed:

@msimberg
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Oct 10, 2022

Build succeeded:

@bors bors bot merged commit a1824c1 into pika-org:main Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msimberg msimberg msimberg approved these changes

@biddisco biddisco Awaiting requested review from biddisco biddisco is a code owner

Assignees

@aurianer aurianer

Labels
category: senders/receivers P2300
Projects
pika planning board
Archived in project
Milestone
0.10.0
Development

Successfully merging this pull request may close these issues.
3 participants
@aurianer @pika-bot @msimberg