Reduce the code complexity by removing some conditional compilation blocks #300
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch ensures that the array descriptor stored in the mpi-caf-token is set to NULL consistently, when no real descriptor is stored in it. Further does the patch ensure freeing memory with gcc-7. The testcases have been actived again for annoucing their passing.
Sync images works by setting up asynchronous receivers for each image in the sync set. Next all images check the image status of all other images participating in the sync. Then each image sends a zero int or the stopped image special code to all other images in the sync set. At last all images wait for the asynchronous receivers to get their data. The race here was, that an image could be in the waiting phase while the stopped image had not set its status correctly yet. The waiting image did not return then, because it never got the stopped image code from the stopped image. To solve this two changes had to be made: 1. caf_finalize() now calls sync_image_internal () 2. After waiting sync_image_internal() checks the status of the image, that send its data, again. sync_image() has be renamed to sync_image_internal(). A flag was added to distinguish calls to sync_image_internal() from caf_finalize and regular sync image calls. The latter shall report an error on failure, while the former keeps silent. This commit fixes the timeout of syncimage_status.f90 mentioned in #298.
This is a pure style/readability change. It consequently makes use of shortcuts for calling locking routines depending on whether they are avaliable. IMO this enhances the readability of the code significantly as conditional compilation blocks strewn around the code are significantly reduced.
Current coverage is 46.28% (diff: 76.59%)@@ master #300 diff @@
==========================================
Files 3 3
Lines 1025 1035 +10
Methods 63 64 +1
Messages 0 0
Branches 197 198 +1
==========================================
+ Hits 466 479 +13
+ Misses 479 476 -3
Partials 80 80
|
To figure why build fails on MaxOS X on Travis add more debugging output to travis build.
Add locking for writing the image status to caf_finalize.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request does not change the semantics of the code, but just makes the code base more readable in fixing some style issues and making use of preprocessor defines instead of preprocessor conditional blocks where needed.
Example:
# ifdef CAF_MPI_LOCK_UNLOCKMPI_Win_lock (MPI_LOCK_EXCLUSIVE, image_index-1, 0, win);# endif // CAF_MPI_LOCK_UNLOCKis replaced by:
CAF_Win_lock (MPI_LOCK_EXCLUSIVE, image_index-1, win);and
# ifdef CAF_MPI_LOCK_UNLOCKMPI_Win_unlock (image_index-1, win);# else // CAF_MPI_LOCK_UNLOCKMPI_Win_flush (image_index-1, win);# endif // CAF_MPI_LOCK_UNLOCKby:
CAF_Win_unlock (image_index-1, win);Which IMHO is far more easier do grasp then the five like for the unlock/flush above. In lines 132 to 146 the defines are set appropriately.