1111 Fix for large message sends #1113

lifflander · 2020-10-14T00:00:34Z

Start with tests that fail for serialized/non-serialized messages
Make a major improvement to how tests are being dispatched. Currently, tests are being ignored due to deficiencies in the gtest cmake. Pull in the latest gtest cmake and fix the bugs in that version of the cmake. This is causing "new" tests to fail on this PR that weren't running before which I am systematically trying to fix
- Addressed & merged in separate PR
Implement "multi-sends" for serialized and non-serialized messages (now passing!)
Add a new runtime argument --vt_max_mpi_send_size=X that defaults to 1ull << 30.
Use the new argument injector to set the size for testing to 16384 bytes

codecov · 2020-10-14T00:17:49Z

Codecov Report

Merging #1113 (90c9305) into develop (f0c4480) will increase coverage by 0.16%.
The diff coverage is 90.81%.

@@             Coverage Diff             @@
##           develop    #1113      +/-   ##
===========================================
+ Coverage    79.28%   79.45%   +0.16%     
===========================================
  Files          716      718       +2     
  Lines        27037    27241     +204     
===========================================
+ Hits         21436    21643     +207     
+ Misses        5601     5598       -3

Impacted Files	Coverage Δ
src/vt/configs/arguments/app_config.h	`100.00% <ø> (ø)`
src/vt/configs/arguments/args.h	`100.00% <ø> (ø)`
src/vt/messaging/irecv_holder.h	`37.50% <0.00%> (+0.35%)`	⬆️
src/vt/rdma/collection/rdma_collection.cc	`1.16% <0.00%> (-0.02%)`	⬇️
...c/vt/serialization/messaging/serialized_data_msg.h	`100.00% <ø> (ø)`
src/vt/utils/memory/memory_units.h	`100.00% <ø> (ø)`
src/vt/rdma/rdma.cc	`25.04% <57.14%> (+0.08%)`	⬆️
src/vt/runtime/runtime_banner.cc	`56.26% <80.00%> (+0.49%)`	⬆️
src/vt/messaging/active.cc	`84.30% <91.52%> (+3.31%)`	⬆️
tests/unit/active/test_active_send_large.cc	`93.02% <93.02%> (ø)`
... and 11 more

lifflander · 2020-10-16T02:46:14Z

Currently failing unrelated tests:

The following tests FAILED:
        248 - vt:TestDiagnosticValue.test_diagnostic_value_2_proc_8 (Failed)
        255 - vt:TestGroup.test_group_range_construct_1_proc_2 (Failed)
        258 - vt:TestGroup.test_group_range_construct_1_proc_4 (Failed)
        282 - vt:*TestLocationRoute/*test_entity_cache_hits_proc_2 (Failed)
        291 - vt:*TestLocationRoute/*test_entity_cache_hits_proc_4 (Failed)
        292 - vt:*TestLocationRoute/*test_entity_cache_migrated_entity_proc_4 (Failed)
        300 - vt:*TestLocationRoute/*test_entity_cache_hits_proc_8 (Failed)
        301 - vt:*TestLocationRoute/*test_entity_cache_migrated_entity_proc_8 (Failed)
        434 - vt:*TestRDMAHandleCollection/*test_rdma_handle_collection_1_proc_2 (Failed)
        440 - vt:*TestRDMAHandleCollection/*test_rdma_handle_collection_1_proc_4 (Failed)
        446 - vt:*TestRDMAHandleCollection/*test_rdma_handle_collection_1_proc_8 (Failed)

Edit: should all be fixed now

src/vt/messaging/active.h

lifflander · 2020-10-16T19:40:59Z

All the test failures should be fixed now.

src/vt/pmpi/generate_mpi_wrappers.pl

src/vt/messaging/active.cc

PhilMiller · 2020-10-27T23:22:43Z

src/vt/messaging/active.cc

+    auto& holder = theEvent()->getEventHolder(ret_event);
+    for (auto&& child_event : events) {
+      holder.get_event()->addEventToList(child_event);
+    }


This block seems a bit awkward, but it's inconsequential

src/vt/messaging/active.cc

PhilMiller · 2020-10-27T23:28:06Z

Reading on my phone, so I can't get a great overview at the moment. I think we're close on this

PhilMiller · 2020-10-28T16:52:00Z

src/vt/configs/arguments/args.cc

@@ -485,6 +485,18 @@ void ArgConfig::addConfigFileArgs(CLI::App& app) {
  a2->group(configGroup);
 }

+void ArgConfig::addRuntimeArgs(CLI::App& app) {


Not a problem, but why is this 'RuntimeArgs' instead of 'MessengerArgs'?

Could be either. I just happened to choose that one.

src/vt/messaging/active.cc

PhilMiller · 2020-10-28T19:11:42Z

At least some of the failing tests are quite real - there's a memory leak reported of an allocation in recvDataDirect. I don't immediately see it in the code, but that needs to be hunted down.

lifflander · 2020-10-28T20:15:42Z

Here is an overview of what got changed by this pull request:

Clones removed
==============
+ src/vt/messaging/active.cc  -2

See the complete overview on Codacy

lifflander requested a review from PhilMiller October 16, 2020 02:43

PhilMiller reviewed Oct 16, 2020

View reviewed changes

src/vt/messaging/active.h Outdated Show resolved Hide resolved

PhilMiller reviewed Oct 16, 2020

View reviewed changes

src/vt/messaging/active.h Outdated Show resolved Hide resolved

PhilMiller reviewed Oct 16, 2020

View reviewed changes

src/vt/messaging/active.h Outdated Show resolved Hide resolved

lifflander marked this pull request as ready for review October 16, 2020 19:41

PhilMiller reviewed Oct 19, 2020

View reviewed changes

src/vt/pmpi/generate_mpi_wrappers.pl Outdated Show resolved Hide resolved

lifflander force-pushed the 1111-over-2gib-send-bug branch 2 times, most recently from db4dc1f to 24d9355 Compare October 20, 2020 21:12

lifflander force-pushed the 1111-over-2gib-send-bug branch from 0af808f to e0f9bc8 Compare October 27, 2020 22:57

lifflander requested review from PhilMiller, JacobDomagala, nlslatt, bradybray, cz4rs, jstrzebonski and nmm0 October 27, 2020 22:59

PhilMiller reviewed Oct 27, 2020

View reviewed changes

src/vt/messaging/active.cc Outdated Show resolved Hide resolved

PhilMiller reviewed Oct 27, 2020

View reviewed changes

src/vt/messaging/active.cc Outdated Show resolved Hide resolved

PhilMiller reviewed Oct 27, 2020

View reviewed changes

src/vt/messaging/active.cc Outdated Show resolved Hide resolved

PhilMiller added the 1.0.0-beta.10.3 label Oct 28, 2020

PhilMiller reviewed Oct 28, 2020

View reviewed changes

src/vt/messaging/active.cc Show resolved Hide resolved

PhilMiller reviewed Oct 28, 2020

View reviewed changes

src/vt/messaging/active.cc Outdated Show resolved Hide resolved

lifflander added 25 commits November 13, 2020 10:01

#1111: tests: split into two sets of types for clarity without string

82dd922

#1111: tests: limit to 2 nodes and less max memory

ca0a947

#1111: config: expand MsgSizeType to 64 bits

e9e3c6e

#1111: active: create function for allocating MPI tags

dc23e36

#1111: active: allow in-progress irecv to not pass request

bce1c42

#1111: active: implement multiple sends/recvs when larger some size

afd7455

#1111: tests: reduce size for serialized test also to fit in memory

6604007

#1111: active: implement cohesive solution for multi-chunk data sends

984a70e

#1111: rdma/serial: adjust APIs to work with new AM sendData/recvData

fd1ced1

#1111: tests: assert that data matches on other end

8a00b97

#1111: active: fix bug in trace build

669ee26

#1111: active: return event and fix managed msg bug

a7de4c8

#1111: active: fix another compile-time tracing bug

2b075e5

#1111: active: simplfy logic for testing

d045cf7

#1111: active: rename "recv_node" to "sender"

b89fe1e

#1111: tests: remove largest sends so CI doesn't timeout

380d473

#1111: args: add an argument for max MPI send size

7e25ace

#1111: test: use new option to set small max msg size for testing

92a67ba

#1111: active: small improvement/refactor

5520039

#1111: active: lift max_per_send out of loops

07789a2

#1111: active: refactor RDMA out of type aliases

0d0d137

#1111: active: fix the remaining deallocation bug

e0aa962

#1111: tests: work around test compile failure w/nvidia

4003573

#1111: active: improve spacing and comments for clarity

cd98980

#1111: active: make allow-mpi calls tigher in blocks

90c9305

lifflander force-pushed the 1111-over-2gib-send-bug branch from aff8bfa to 90c9305 Compare November 13, 2020 18:01

cz4rs approved these changes Nov 13, 2020

View reviewed changes

lifflander merged commit 9dd92a0 into develop Nov 13, 2020

PhilMiller mentioned this pull request Jan 5, 2021

1197: prepare beta-10.4 release #1198

Merged

cz4rs mentioned this pull request Jun 15, 2021

move vt version number into a separate file #1467

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1111 Fix for large message sends #1113

1111 Fix for large message sends #1113

lifflander commented Oct 14, 2020 •

edited

Loading

codecov bot commented Oct 14, 2020 •

edited

Loading

lifflander commented Oct 16, 2020 •

edited

Loading

lifflander commented Oct 16, 2020

PhilMiller Oct 27, 2020

PhilMiller commented Oct 27, 2020

PhilMiller Oct 28, 2020

lifflander Nov 3, 2020

PhilMiller commented Oct 28, 2020

lifflander commented Oct 28, 2020

1111 Fix for large message sends #1113

1111 Fix for large message sends #1113

Conversation

lifflander commented Oct 14, 2020 • edited Loading

codecov bot commented Oct 14, 2020 • edited Loading

Codecov Report

lifflander commented Oct 16, 2020 • edited Loading

lifflander commented Oct 16, 2020

PhilMiller Oct 27, 2020

Choose a reason for hiding this comment

PhilMiller commented Oct 27, 2020

PhilMiller Oct 28, 2020

Choose a reason for hiding this comment

lifflander Nov 3, 2020

Choose a reason for hiding this comment

PhilMiller commented Oct 28, 2020

lifflander commented Oct 28, 2020

lifflander commented Oct 14, 2020 •

edited

Loading

codecov bot commented Oct 14, 2020 •

edited

Loading

lifflander commented Oct 16, 2020 •

edited

Loading