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

#2001: Avoid reconstruction when pre-constructed elements are present in collection #2004

Merged
merged 4 commits into from
Nov 9, 2022
Merged

#2001: Avoid reconstruction when pre-constructed elements are present in collection #2004

merged 4 commits into from
Nov 9, 2022

Conversation

thearusable
Copy link
Contributor

@thearusable thearusable commented Oct 31, 2022 •
edited
Loading

Fixes #2001

@thearusable thearusable self-assigned this Oct 31, 2022
@github-actions
Copy link

github-actions bot commented Oct 31, 2022 •
edited
Loading

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-6, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-3.9, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-5, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-5.0, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-9, ubuntu, mpich, zoltan)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-9, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, alpine, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 10.1, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-11, ubuntu, mpich, json schema test)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-7, ubuntu, mpich, trace runtime, LB)

Build for 2be1f98 (2022-11-09 18:32:57 UTC)

Compilation - successful

Testing - passed

Build log

@thearusable thearusable changed the title #2001: Add UT to replicate defect #2001: Remove reconstruction when pre-constructed elements are present in collection Nov 4, 2022
@thearusable thearusable changed the title #2001: Remove reconstruction when pre-constructed elements are present in collection #2001: Avoid reconstruction when pre-constructed elements are present in collection Nov 4, 2022
@thearusable thearusable marked this pull request as ready for review November 4, 2022 16:15
nlslatt
nlslatt reviewed Nov 4, 2022
View reviewed changes
src/vt/vrt/collection/collection_builder.impl.h Outdated
global_constructed_elms++;
});
}
if (po.list_insert_here_.empty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it might be possible to mix .listInsertHere with .bulkInsert and/or .listInsert. @lifflander?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, it is possible to mix .listInsertHere with .listInsert. I updated the if statement to reflect that

@thearusable thearusable requested a review from nlslatt November 8, 2022 09:08
@codecov
Copy link

codecov bot commented Nov 9, 2022 •
edited
Loading

Codecov Report

Merging #2004 (2be1f98) into develop (bf4eeba) will increase coverage by 0.07%.
The diff coverage is 98.64%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2004      +/-   ##
===========================================
+ Coverage    84.39%   84.46%   +0.07%     
===========================================
  Files          730      730              
  Lines        25633    25772     +139     
===========================================
+ Hits         21632    21769     +137     
- Misses        4001     4003       +2
Impacted Files Coverage Δ
tests/unit/collection/test_list_insert.cc 99.12% <98.50%> (-0.88%) ⬇️
src/vt/vrt/collection/collection_builder.impl.h 96.51% <100.00%> (+0.08%) ⬆️
src/vt/vrt/collection/param/construct_params.h 96.92% <0.00%> (+0.14%) ⬆️

nlslatt
nlslatt approved these changes Nov 9, 2022
View reviewed changes
Copy link
Collaborator

@nlslatt nlslatt 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

@nlslatt nlslatt merged commit f635bd2 into develop Nov 9, 2022
@nlslatt nlslatt mentioned this pull request Nov 9, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlslatt nlslatt nlslatt approved these changes

@lifflander lifflander Awaiting requested review from lifflander

Assignees

@thearusable thearusable

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When a collection is list inserted with pre-constructed elements don't try to construct
2 participants
@thearusable @nlslatt