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

mpi: manually heap-allocate payloads for local messages #378

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

csegarragonz
Copy link
Collaborator

@csegarragonz csegarragonz commented Feb 15, 2024

This PR makes sure that, when sending a local MPI message, we make the minimum number of copies (of the message payload).

The minimum number of copies is two: (i) from the sender-owned buffer, to a runtime-owned buffer, and (ii) from the runtime-owned buffer to the receiver-owned buffer.

In the future, we could consider using something different than std::malloc.

@lgarithm
Copy link
Contributor

TODO:

  • use memory pool instead of malloc and free.

@@ -242,21 +242,6 @@ TEST_CASE_METHOD(MpiTestFixture, "Test send and recv on same host", "[mpi]")
world.send(
rankA1, rankA2, BYTES(messageData.data()), MPI_INT, messageData.size());

SECTION("Test queueing")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does not apply as the message buffer is not in the queue any more.

@csegarragonz csegarragonz changed the title mpi: make local fast path faster mpi: manually heap-allocate payloads for local messages Feb 26, 2024
@csegarragonz csegarragonz force-pushed the local-fastpath branch 2 times, most recently from 7ec2294 to 65cd6af Compare February 26, 2024 13:48
Copy link

codecov bot commented Feb 27, 2024

Codecov Report

Attention: Patch coverage is 85.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 80.86%. Comparing base (b31abc1) to head (0895246).
Report is 6 commits behind head on main.

Files Patch % Lines
src/mpi/MpiWorld.cpp 82.35% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #378      +/-   ##
==========================================
- Coverage   81.94%   80.86%   -1.08%     
==========================================
  Files         106      107       +1     
  Lines        8013     7222     -791     
==========================================
- Hits         6566     5840     -726     
+ Misses       1447     1382      -65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@csegarragonz csegarragonz merged commit 7483943 into main Feb 27, 2024
11 of 12 checks passed
@csegarragonz csegarragonz deleted the local-fastpath branch February 27, 2024 11:51
csegarragonz added a commit that referenced this pull request Feb 28, 2024
* mpi: make local fast path faster

* util/memory: use mimalloc

* nits: run clang-format

* nits: some self-review

* mpi: fix detection of local messages in receiver

* mpi: only memcpy the required bytes

* tests: make lsan happy with the malloc/free optimization

* nits: run clang format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants