[xuyao00]

Based on #559: this PR also shows #559's 3 commits until #559 is merged. Review only the last 4 commits.

The last step of the cleanup: the source layout and two renames. No code changes.

• mpi-proxy-split/ holds MANA's sources, its tests and its example. Its parts move to top-level directories: src/ (with lower-half/ and mpi-wrappers/), test/ and examples/. The paths in the Makefiles, configure, CI, the style hook, the test scripts, the README and the man page are updated. The top-level Makefile now runs make tests and make check in test/ and examples/. The rules of mpi-proxy-split/.gitignore move to the top-level .gitignore.
• The two benchmarks, virtid_bench and synthetic_app, move from test/ to bench/. bench/Makefile.in has the rule that built them in test/Makefile.in, and make tests still builds them.
• p2p_drain_send_recv.{cpp,h} is renamed p2p_drain.{cpp,h}.
• p2p_log_replay.{cpp,h} is folded into other files, with no change to the code. The pending-call list, mpi_message_t and replayMpiP2pOnRestart() move to p2p_drain. The world rank and size, getLocalRankInfo() and updateCkptDirByRank() move to mpi_plugin.

After pulling this change, run ./configure again: it makes the Makefiles in the new directories.

Left as they are:

• the barrier name "MPI:p2p_log_replay.cpp-void";
• p2p_drain.h's header guard, _P2P_SEND_RECV_H;
• two mentions of mpi-proxy-split in CHANGELOG.md: the 1.5.0 notes, and an example in the release checklist that names a file that does not exist.

The wiki and readthedocs pages also give mpi-proxy-split/ paths.

The pre-commit hook rejects commit 1. The hook runs git diff-index --check without rename detection, so it reports the trailing whitespace that the moved files already have. With -M, the check passes. No added or modified file in commit 1 is C++, so the hook's style check has nothing to check. Commits 2 to 4 pass the hook.

Checks (each commit built from clean, compared with #559's build; the comparison leaves out debug info and the build-ID):

• Commits 1 and 2: the 112 outputs (objects, archives, libraries, the lower half, the test, example and benchmark programs) are the same as Install MANA with make install #559's, at their new paths.
• Commit 3: the outputs are the same, except that p2p_drain.o and libmana.so have __FILE__ strings with the new file name. Apart from addresses, their instructions are the same.
• Commit 4: p2p_drain.o, mpi_plugin.o and libmana.so change, because GCC inlines library code (std::string, std::map, DMTCP's allocator) differently in the larger files. The moved MANA functions make the same calls as before. libmana.so exports two more weak symbols: two instances of std::string's constructor from const char*.
• Tests (locally, MPICH 5.0.1): test-mana-restart.py and test-loader-debug.py pass; make check passed 53 of 53.
• Install: after ./configure --prefix=... and make install, autotest.py --mana-root on the prefix passes 3 of 3 with the source tree's bin/ and lib/ moved away. make uninstall removes every MANA file (DMTCP's part fails until Update DMTCP for its make uninstall fix #561 is merged; with Update DMTCP for its make uninstall fix #561, every installed file is removed). make distclean removes every Makefile that configure makes, including the four in the new directories.

[coderabbitai]

Important

Review skipped

Too many files!

This PR contains 128 files, which is 28 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

Check out review usage here.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 04edaf21-f407-419f-9220-208e3ed06216

Commits

Reviewing files that changed from the base of the PR and between 6ceb65f and 480c248.

Files selected for processing: 128

• .github/workflows/mana-tests.yml
• .gitignore
• Makefile.in
• README.md
• bench/Makefile.in
• bench/README-synthetic_app.md
• bench/synthetic_app.c
• bench/virtid_bench.c
• configure
• configure.ac
• examples/Makefile.in
• examples/ring.c
• manpages/mana.1
• manpages/mana.1.md
• mpi-proxy-split/.gitignore
• mpi-proxy-split/p2p_log_replay.cpp
• mpi-proxy-split/p2p_log_replay.h
• src/Makefile.in
• src/lower-half/Makefile.in
• src/lower-half/README
• src/lower-half/build-id-debug.cpp
• src/lower-half/build-id-debug.h
• src/lower-half/copy-stack.c
• src/lower-half/get-symbol-offset.c
• src/lower-half/lh-func-ptr.c
• src/lower-half/logging.h
• src/lower-half/lower-half-api.h
• src/lower-half/lower-half.cpp
• src/lower-half/mem-wrapper.cpp
• src/lower-half/mem-wrapper.h
• src/lower-half/mmap-fixed-noreplace.c
• src/lower-half/patch-trampoline.c
• src/lower-half/patch-trampoline.h
• src/lower-half/switch-context.cpp
• src/lower-half/switch-context.h
• src/lower_half_ckpt.cpp
• src/lower_half_ckpt.h
• src/mpi-wrappers/Makefile.in
• src/mpi-wrappers/fortran_constants.f90
• src/mpi-wrappers/fortran_handles.h
• src/mpi-wrappers/generate-mpi-fortran-wrappers.py
• src/mpi-wrappers/generate-mpi-stub-objects.py
• src/mpi-wrappers/generate-mpi-stub-wrappers.py
• src/mpi-wrappers/generate-mpi-unimplemented-wrappers.py
• src/mpi-wrappers/get_fortran_constants.c
• src/mpi-wrappers/mpi_cart_wrappers.cpp
• src/mpi-wrappers/mpi_collective_wrappers.cpp
• src/mpi-wrappers/mpi_comm_wrappers.cpp
• src/mpi-wrappers/mpi_error_wrappers.cpp
• src/mpi-wrappers/mpi_file_wrappers.cpp
• src/mpi-wrappers/mpi_fortran_attr_wrappers.cpp
• src/mpi-wrappers/mpi_fortran_wrappers.txt
• src/mpi-wrappers/mpi_group_wrappers.cpp
• src/mpi-wrappers/mpi_nextfunc.h
• src/mpi-wrappers/mpi_op_wrappers.cpp
• src/mpi-wrappers/mpi_p2p_wrappers.cpp
• src/mpi-wrappers/mpi_request_wrappers.cpp
• src/mpi-wrappers/mpi_stub_audit.c
• src/mpi-wrappers/mpi_stub_data.c
• src/mpi-wrappers/mpi_stub_wrappers.txt
• src/mpi-wrappers/mpi_t_wrappers.cpp
• src/mpi-wrappers/mpi_type_wrappers.cpp
• src/mpi-wrappers/mpi_unimplemented_wrappers.txt
• src/mpi-wrappers/mpi_win_wrappers.cpp
• src/mpi-wrappers/mpi_wrappers.cpp
• src/mpi_files.h
• src/mpi_plugin.cpp
• src/mpi_plugin.h
• src/p2p_drain.cpp
• src/p2p_drain.h
• src/seq_num.cpp
• src/seq_num.h
• src/uh_wrappers.cpp
• src/uh_wrappers.h
• src/virtual_id.cpp
• src/virtual_id.h
• test/Makefile.in
• test/abort.c
• test/attributes.c
• test/autotest.py
• test/cartesian.c
• test/collectives.c
• test/comm_create_group.c
• test/comm_free.c
• test/comm_free_order.c
• test/comm_self.c
• test/communicators.c
• test/datatypes.c
• test/dlopen_mpi.c
• test/exit_after_finalize.c
• test/finalize_unsync.c
• test/fortran_attributes.f90
• test/fortran_handles.f90
• test/fortran_nonblocking.f90
• test/fortran_p2p.f90
• test/init_thread.c
• test/keyvals.c
• test/loader-debug-driver.cpp
• test/mana_test.h
• test/memory.c
• test/misc_calls.c
• test/mmap_threads.c
• test/nbc_order.c
• test/nonblocking_collectives.c
• test/p2p_any_source.c
• test/p2p_blocking.c
• test/p2p_issend_comm_free.c
• test/p2p_issend_envelope.c
• test/p2p_issend_nbx.c
• test/p2p_issend_twice.c
• test/p2p_large.c
• test/p2p_nonblocking.c
• test/p2p_probe.c
• test/p2p_proc_null.c
• test/p2p_ring.c
• test/p2p_ssend.c
• test/p2p_ssend_blocked.c
• test/p2p_ssend_collective.c
• test/p2p_ssend_comm.c
• test/p2p_ssend_order.c
• test/p2p_ssend_probe.c
• test/p2p_ssend_sync.c
• test/p2p_ssend_wildcard.c
• test/realloc_grow.c
• test/shared_memory.c
• test/test-loader-debug.py
• test/test-mana-restart.py
• util/hooks/pre-commit

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[gc00]

LGTM