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

#1009 add memory footprinting support for VT components #1013

Merged
merged 98 commits into from
Nov 25, 2020
Merged

#1009 add memory footprinting support for VT components #1013

merged 98 commits into from
Nov 25, 2020

Conversation

cz4rs
Copy link
Contributor

@cz4rs cz4rs commented Sep 1, 2020

fixes #1009

@cz4rs
Copy link
Contributor Author

cz4rs commented Sep 1, 2020 •
edited
Loading

pushed 6a34657 as a basis for discussion

- based on sizer test, serialize method needs to be implemented in all components (and the types they are composed of)
- is there any need for using isByteCopyable = std::false_type;?

@codecov
Copy link

codecov bot commented Sep 1, 2020 •
edited
Loading

Codecov Report

Merging #1013 (f424afb) into develop (fd29fe7) will increase coverage by 0.03%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1013      +/-   ##
===========================================
+ Coverage    79.54%   79.57%   +0.03%     
===========================================
  Files          719      723       +4     
  Lines        27326    27774     +448     
===========================================
+ Hits         21736    22101     +365     
- Misses        5590     5673      +83
Impacted Files Coverage Δ
src/vt/group/group_info.h 50.00% <0.00%> (-50.00%) ⬇️
src/vt/messaging/listener.h 66.66% <0.00%> (-33.34%) ⬇️
src/vt/rdma/channel/rdma_channel.h 0.00% <0.00%> (ø)
src/vt/rdma/channel/rdma_channel_lookup.h 60.00% <0.00%> (-15.00%) ⬇️
src/vt/rdma/group/rdma_group.h 0.00% <0.00%> (ø)
src/vt/rdma/group/rdma_map.h 31.25% <0.00%> (-7.22%) ⬇️
src/vt/rdma/rdma_info.h 62.50% <0.00%> (-37.50%) ⬇️
src/vt/rdma/rdma_pending.h 53.84% <0.00%> (-16.16%) ⬇️
src/vt/rdma/state/rdma_state.h 12.50% <0.00%> (-87.50%) ⬇️
src/vt/rdmahandle/holder.h 57.14% <0.00%> (-42.86%) ⬇️
... and 95 more

@lifflander lifflander mentioned this pull request Sep 1, 2020
Open
@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch from 6a34657 to e5b91fe Compare September 8, 2020 15:09
@cz4rs
Copy link
Contributor Author

cz4rs commented Sep 8, 2020

This depends on following PR in checkpoint library:
DARMA-tasking/magistrate#124

@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch from e5b91fe to 2d3b91a Compare September 9, 2020 23:38
@cz4rs cz4rs mentioned this pull request Sep 9, 2020
Merged
@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch 4 times, most recently from 69c1637 to 5e4a767 Compare September 16, 2020 16:29
@cz4rs cz4rs changed the title #1009 add serialize method for ArgConfig component #1009 add memory footprinting support for components Sep 16, 2020
@cz4rs cz4rs changed the title #1009 add memory footprinting support for components #1009 add memory footprinting support for VT components Sep 16, 2020
@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch 5 times, most recently from 248c74e to 35471d6 Compare September 17, 2020 19:52
@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch from 35471d6 to b3ca72d Compare September 17, 2020 23:29
@lifflander
Copy link
Collaborator

The corresponding PR in checkpoint: DARMA-tasking/magistrate#124 has now been merged!

@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch 7 times, most recently from ca6a53a to 6d71f6c Compare September 22, 2020 09:07
cz4rs added 16 commits November 21, 2020 01:43
@cz4rs
#1009: memory footprinting: clear out removed variables after rebase
a7d548b
@cz4rs
#1009: memory footprinting: add support for component::Diagnostic
bb25f07
@cz4rs
#1009: memory footprinting: improve code coverage
505a057
@cz4rs
#1009: memory footprinting: remove unnnecessary includes
171083d 
Remove unnecessary checkpoint headers and enabe_ifs (virtual serialization
leftovers).
@cz4rs
#1009: memory footprinting: enable printing by default
a0a1384 
Verify no errors in CI with memory footprinting turned on by default.
Note: switch back to disabled by default before merging.
@cz4rs
#1009: memory footprinting: disable printing by default
2e699ca
@cz4rs
#1009: memory footprinting: formatting corrections
101a365 
- use standard formatting
- remove superfluous blank lines
@cz4rs
#1009: memory footprinting: display warning for unsupported components
79728ce
@cz4rs
#1009: memory footprinting: improve formatting and units
92db8e8
@cz4rs
#1009: memory footprinting: improve test coverage
9cbdd9a
@cz4rs
#1009: memory footprinting: improve MsgSharedPtr support
52e25f8
@cz4rs
#1009: memory footprinting: limit MsgSharedPtr serializer to footprin…
24ee3af 
…ting
@cz4rs
#1009: memory footprinting: add missing members to serializers
809f826 
Add any missing members to existing serializers and add serializers for
classes involved.
@cz4rs
#1009: memory footprinting: add serializer for DiagnosticStatsPack
df266a4 
Add serializer for DiagnosticStatsPack and use it in derived classes.
@cz4rs
#1009: memory footprinting: use branch for checkpoint
a292ff4
@cz4rs
#1009: memory footprinting: use | operator for MPI types
fccc102
@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch from dcbbd26 to 951f236 Compare November 21, 2020 00:46
@cz4rs cz4rs force-pushed the 1009-components-memory-usage branch from 951f236 to 852a74e Compare November 24, 2020 13:59
@cz4rs
Copy link
Contributor Author

cz4rs commented Nov 24, 2020 •
edited
Loading

this is passing the tests just fine, but it needs to have a292ff4 reverted (pointing checkpoint_rev back to develop) before merging (once DARMA-tasking/magistrate#157 gets merged)

@cz4rs cz4rs marked this pull request as draft November 24, 2020 19:20
PhilMiller
PhilMiller approved these changes Nov 24, 2020
View reviewed changes
Copy link
Member

@PhilMiller PhilMiller left a comment

Choose a reason for hiding this comment

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

Other than the location lookup cache, this all looks good to me.

@cz4rs cz4rs marked this pull request as ready for review November 25, 2020 06:41
@cz4rs cz4rs merged commit 65408f6 into develop Nov 25, 2020
@cz4rs cz4rs mentioned this pull request Jun 15, 2021
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lifflander lifflander lifflander approved these changes

@PhilMiller PhilMiller PhilMiller approved these changes

@pnstickne pnstickne Awaiting requested review from pnstickne

@JacobDomagala JacobDomagala Awaiting requested review from JacobDomagala

@jstrzebonski jstrzebonski Awaiting requested review from jstrzebonski

@bradybray bradybray Awaiting requested review from bradybray

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Monitor memory usage of VT components
4 participants
@cz4rs @lifflander @PhilMiller @pnstickne