Commit 5af1d475abf77f466a66bddaa70b339ea1ff1d65
1 parent
92543c1e
Change critical zones in GWcs::dir2pix and GWcs::pix2dir to remove critical regi…
…ons and replace with omp_locks
Showing
2 changed files
with
79 additions
and
6 deletions
include/GWcs.hpp
| @@ -30,6 +30,7 @@ | @@ -30,6 +30,7 @@ | ||
| 30 | /* __ Includes ___________________________________________________________ */ | 30 | /* __ Includes ___________________________________________________________ */ |
| 31 | #include <vector> | 31 | #include <vector> |
| 32 | #include <string> | 32 | #include <string> |
| 33 | +#include <map> | ||
| 33 | #include <iostream> | 34 | #include <iostream> |
| 34 | #include "GSkyProjection.hpp" | 35 | #include "GSkyProjection.hpp" |
| 35 | #include "GLog.hpp" | 36 | #include "GLog.hpp" |
| @@ -37,6 +38,10 @@ | @@ -37,6 +38,10 @@ | ||
| 37 | #include "GSkyDir.hpp" | 38 | #include "GSkyDir.hpp" |
| 38 | #include "GSkyPixel.hpp" | 39 | #include "GSkyPixel.hpp" |
| 39 | 40 | ||
| 41 | +#ifdef _OPENMP | ||
| 42 | +#include <omp.h> | ||
| 43 | +#endif | ||
| 44 | + | ||
| 40 | 45 | ||
| 41 | /***********************************************************************//** | 46 | /***********************************************************************//** |
| 42 | * @class GWcs | 47 | * @class GWcs |
| @@ -220,6 +225,15 @@ protected: | @@ -220,6 +225,15 @@ protected: | ||
| 220 | mutable GSkyDir m_last_dir2pix_dir; //!< Last sky direction for dir2pix | 225 | mutable GSkyDir m_last_dir2pix_dir; //!< Last sky direction for dir2pix |
| 221 | mutable GSkyPixel m_last_pix2dir_pix; //!< Last pixel for pix2dir | 226 | mutable GSkyPixel m_last_pix2dir_pix; //!< Last pixel for pix2dir |
| 222 | mutable GSkyPixel m_last_dir2pix_pix; //!< Last pixel for dir2pix | 227 | mutable GSkyPixel m_last_dir2pix_pix; //!< Last pixel for dir2pix |
| 228 | + | ||
| 229 | + // Thread locking | ||
| 230 | + void init_lock(const int& lock_id=0) const; | ||
| 231 | + void set_lock(const int& lock_id=0) const; | ||
| 232 | + void unset_lock(const int& lock_id=0) const; | ||
| 233 | + | ||
| 234 | + #ifdef _OPENMP | ||
| 235 | + mutable std::map<int, omp_lock_t> m_locks; | ||
| 236 | + #endif | ||
| 223 | }; | 237 | }; |
| 224 | 238 | ||
| 225 | 239 | ||
| @@ -236,4 +250,49 @@ int GWcs::size(void) const | @@ -236,4 +250,49 @@ int GWcs::size(void) const | ||
| 236 | return 2; | 250 | return 2; |
| 237 | } | 251 | } |
| 238 | 252 | ||
| 253 | + | ||
| 254 | +/***********************************************************************//** | ||
| 255 | + * @brief Initializes an OpenMP lock with a specific name | ||
| 256 | + * | ||
| 257 | + * @param[in] lock_name Name of the lock that should be initialized | ||
| 258 | + ***************************************************************************/ | ||
| 259 | +inline | ||
| 260 | +void GWcs::init_lock(const int& lock_id) const | ||
| 261 | +{ | ||
| 262 | + #ifdef _OPENMP | ||
| 263 | + if (m_locks.count(lock_id) == 0) { | ||
| 264 | + m_locks[lock_id] = omp_lock_t(); | ||
| 265 | + omp_init_lock(&m_locks[lock_id]); | ||
| 266 | + } | ||
| 267 | + #endif | ||
| 268 | +} | ||
| 269 | + | ||
| 270 | + | ||
| 271 | +/***********************************************************************//** | ||
| 272 | + * @brief Sets an OpenMP lock with a specific name | ||
| 273 | + * | ||
| 274 | + * @param[in] lock_name Name of the lock that should be set | ||
| 275 | + ***************************************************************************/ | ||
| 276 | +inline | ||
| 277 | +void GWcs::set_lock(const int& lock_id) const | ||
| 278 | +{ | ||
| 279 | + #ifdef _OPENMP | ||
| 280 | + omp_set_lock(&m_locks[lock_id]); | ||
| 281 | + #endif | ||
| 282 | +} | ||
| 283 | + | ||
| 284 | + | ||
| 285 | +/***********************************************************************//** | ||
| 286 | + * @brief Releases a previously set OpenMP lock | ||
| 287 | + * | ||
| 288 | + * @param[in] lock_name Name of the lock to be released | ||
| 289 | + ***************************************************************************/ | ||
| 290 | +inline | ||
| 291 | +void GWcs::unset_lock(const int& lock_id) const | ||
| 292 | +{ | ||
| 293 | + #ifdef _OPENMP | ||
| 294 | + omp_unset_lock(&m_locks[lock_id]); | ||
| 295 | + #endif | ||
| 296 | +} | ||
| 297 | + | ||
| 239 | #endif /* GWCS_HPP */ | 298 | #endif /* GWCS_HPP */ |
src/sky/GWcs.cpp
| @@ -678,13 +678,16 @@ GSkyDir GWcs::pix2dir(const GSkyPixel& pixel) const | @@ -678,13 +678,16 @@ GSkyDir GWcs::pix2dir(const GSkyPixel& pixel) const | ||
| 678 | // the one used for the cache, then get the cached sky direction. We need | 678 | // the one used for the cache, then get the cached sky direction. We need |
| 679 | // to put this in a OMP critical zone so that no other thread will change | 679 | // to put this in a OMP critical zone so that no other thread will change |
| 680 | // the cache in the meantime | 680 | // the cache in the meantime |
| 681 | - #pragma omp critical(GWcs_pix2dir_1) | 681 | + bool update_cache = true; |
| 682 | + set_lock(2); | ||
| 682 | if (m_has_pix2dir_cache && (m_last_pix2dir_pix == pixel)) { | 683 | if (m_has_pix2dir_cache && (m_last_pix2dir_pix == pixel)) { |
| 683 | dir = m_last_pix2dir_dir; | 684 | dir = m_last_pix2dir_dir; |
| 685 | + update_cache = false; | ||
| 684 | } | 686 | } |
| 687 | + unset_lock(2); | ||
| 685 | 688 | ||
| 686 | // ... otherwise the computation of the sky direction is needed | 689 | // ... otherwise the computation of the sky direction is needed |
| 687 | - else { | 690 | + if (update_cache) { |
| 688 | 691 | ||
| 689 | // Allocate memory for transformation | 692 | // Allocate memory for transformation |
| 690 | double pixcrd[2]; | 693 | double pixcrd[2]; |
| @@ -712,12 +715,13 @@ GSkyDir GWcs::pix2dir(const GSkyPixel& pixel) const | @@ -712,12 +715,13 @@ GSkyDir GWcs::pix2dir(const GSkyPixel& pixel) const | ||
| 712 | 715 | ||
| 713 | // Store result in cache. We need to put this in a OMP critical zone | 716 | // Store result in cache. We need to put this in a OMP critical zone |
| 714 | // to make sure that no other thread is fiddeling with the cache | 717 | // to make sure that no other thread is fiddeling with the cache |
| 715 | - #pragma omp critical(GWcs_pix2dir_2) | 718 | + set_lock(2); |
| 716 | { | 719 | { |
| 717 | m_has_pix2dir_cache = true; | 720 | m_has_pix2dir_cache = true; |
| 718 | m_last_pix2dir_pix = pixel; | 721 | m_last_pix2dir_pix = pixel; |
| 719 | m_last_pix2dir_dir = dir; | 722 | m_last_pix2dir_dir = dir; |
| 720 | } | 723 | } |
| 724 | + unset_lock(2); | ||
| 721 | 725 | ||
| 722 | // Debug: Dump transformation steps | 726 | // Debug: Dump transformation steps |
| 723 | #if defined(G_XY2DIR_DEBUG) | 727 | #if defined(G_XY2DIR_DEBUG) |
| @@ -758,13 +762,16 @@ GSkyPixel GWcs::dir2pix(const GSkyDir& dir) const | @@ -758,13 +762,16 @@ GSkyPixel GWcs::dir2pix(const GSkyDir& dir) const | ||
| 758 | // to the one used for the cache, then get the cached sky pixel. We need | 762 | // to the one used for the cache, then get the cached sky pixel. We need |
| 759 | // to put this in a OMP critical zone so that no other thread will change | 763 | // to put this in a OMP critical zone so that no other thread will change |
| 760 | // the cache in the meantime | 764 | // the cache in the meantime |
| 761 | - #pragma omp critical(GWcs_dir2pix_1) | 765 | + bool update_cache = true; |
| 766 | + set_lock(1); | ||
| 762 | if (m_has_dir2pix_cache && (m_last_dir2pix_dir == dir)) { | 767 | if (m_has_dir2pix_cache && (m_last_dir2pix_dir == dir)) { |
| 763 | pixel = m_last_dir2pix_pix; | 768 | pixel = m_last_dir2pix_pix; |
| 769 | + update_cache = false; | ||
| 764 | } | 770 | } |
| 771 | + unset_lock(1); | ||
| 765 | 772 | ||
| 766 | // ... otherwise the computation of the sky pixel is needed | 773 | // ... otherwise the computation of the sky pixel is needed |
| 767 | - else { | 774 | + if (update_cache) { |
| 768 | 775 | ||
| 769 | // Allocate memory for transformation | 776 | // Allocate memory for transformation |
| 770 | double pixcrd[2]; | 777 | double pixcrd[2]; |
| @@ -793,12 +800,13 @@ GSkyPixel GWcs::dir2pix(const GSkyDir& dir) const | @@ -793,12 +800,13 @@ GSkyPixel GWcs::dir2pix(const GSkyDir& dir) const | ||
| 793 | 800 | ||
| 794 | // Store result in cache. We need to put this in a OMP critical zone | 801 | // Store result in cache. We need to put this in a OMP critical zone |
| 795 | // to make sure that no other thread is fiddeling with the cache | 802 | // to make sure that no other thread is fiddeling with the cache |
| 796 | - #pragma omp critical(GWcs_dir2pix_2) | 803 | + set_lock(1); |
| 797 | { | 804 | { |
| 798 | m_has_dir2pix_cache = true; | 805 | m_has_dir2pix_cache = true; |
| 799 | m_last_dir2pix_dir = dir; | 806 | m_last_dir2pix_dir = dir; |
| 800 | m_last_dir2pix_pix = pixel; | 807 | m_last_dir2pix_pix = pixel; |
| 801 | } | 808 | } |
| 809 | + unset_lock(1); | ||
| 802 | 810 | ||
| 803 | // Debug: Dump transformation steps | 811 | // Debug: Dump transformation steps |
| 804 | #if defined(G_DIR2XY_DEBUG) | 812 | #if defined(G_DIR2XY_DEBUG) |
| @@ -944,6 +952,12 @@ void GWcs::init_members(void) | @@ -944,6 +952,12 @@ void GWcs::init_members(void) | ||
| 944 | m_last_pix2dir_pix.clear(); | 952 | m_last_pix2dir_pix.clear(); |
| 945 | m_last_dir2pix_pix.clear(); | 953 | m_last_dir2pix_pix.clear(); |
| 946 | 954 | ||
| 955 | + // Initialize the locks | ||
| 956 | + #ifdef _OPENMP | ||
| 957 | + init_lock(1); | ||
| 958 | + init_lock(2); | ||
| 959 | + #endif | ||
| 960 | + | ||
| 947 | // Return | 961 | // Return |
| 948 | return; | 962 | return; |
| 949 | } | 963 | } |