Skip to content

Commit 9be7101

Browse files
authored
Merge pull request #1894 from llnl/bugfix/whitlock/axom_copy_umpire
Fix axom::copy so it can be called from OpenMP loop
2 parents 81611d9 + 87a6b29 commit 9be7101

4 files changed

Lines changed: 109 additions & 3 deletions

File tree

RELEASE-NOTES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ The Axom project release numbers follow [Semantic Versioning](http://semver.org/
6565
- Core: A moved-from Array is valid, e.g. it can be pushed to
6666
- Core: Improved `axom::FlatMap` insertion performance by fusing duplicate-key lookup with empty-slot probing.
6767
- Core: Updated DeviceHash to use 64-bit hash results and improved coverage for integer and floating-point hashing.
68+
- Core: Avoids a first-use race in `axom::copy()` when multiple OpenMP threads concurrently trigger Umpire fallback host-copy initialization.
6869
- Python: Improves lifetime handling for python wrapped sidre entities, including support for external views into numpy arrays.
6970

7071
## [Version 0.14.0] - Release date 2026-03-31

src/axom/core/memory_management.hpp

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,49 @@
2727
#include <cstddef>
2828
#include <cstdlib>
2929
#include <iostream>
30+
#include <mutex>
3031
#include <string>
3132
#include <type_traits>
3233

3334
namespace axom
3435
{
36+
#ifdef AXOM_USE_UMPIRE
37+
namespace detail
38+
{
39+
/*!
40+
* \brief Cache for Umpire data used in axom::copy.
41+
*/
42+
struct UmpireCopyContext
43+
{
44+
umpire::strategy::AllocationStrategy* hostStrategy {nullptr};
45+
umpire::op::MemoryOperationRegistry* operationRegistry {nullptr};
46+
};
47+
48+
/*!
49+
* \brief Gets a reference to an UmpireCopyContext object, initializing it on demand.
50+
* The static UmpireCopyContext is initialized via std::call_once so multiple
51+
* threads can call this function and only initialize the object once.
52+
*
53+
* \return A reference to the cached UmpireCopyContext.
54+
*/
55+
inline const UmpireCopyContext& getUmpireCopyContext() noexcept
56+
{
57+
static std::once_flag once;
58+
static UmpireCopyContext context {};
59+
60+
// Resolve Umpire's fallback HOST path once so the first threaded axom::copy()
61+
// cannot race through lazy resource creation.
62+
std::call_once(once, []() {
63+
auto& rm = umpire::ResourceManager::getInstance();
64+
context.hostStrategy = rm.getAllocator("HOST").getAllocationStrategy();
65+
context.operationRegistry = &umpire::op::MemoryOperationRegistry::getInstance();
66+
});
67+
68+
return context;
69+
}
70+
} // namespace detail
71+
#endif
72+
3573
// To co-exist with Umpire allocator ids, use negative values here.
3674
constexpr int INVALID_ALLOCATOR_ID = -1; //!< Place holder for no/unknown allocator
3775
constexpr int MALLOC_ALLOCATOR_ID = -3; //!< Refers to MemorySpace::Malloc
@@ -462,11 +500,11 @@ inline T* reallocate(T* pointer, std::size_t n, int allocID) noexcept
462500
inline void copy(void* dst, const void* src, std::size_t numbytes) noexcept
463501
{
464502
#ifdef AXOM_USE_UMPIRE
503+
const auto& copyContext = detail::getUmpireCopyContext();
465504
umpire::ResourceManager& rm = umpire::ResourceManager::getInstance();
466-
umpire::op::MemoryOperationRegistry& op_registry =
467-
umpire::op::MemoryOperationRegistry::getInstance();
505+
umpire::op::MemoryOperationRegistry& op_registry = *copyContext.operationRegistry;
468506

469-
auto dstStrategy = rm.getAllocator("HOST").getAllocationStrategy();
507+
auto dstStrategy = copyContext.hostStrategy;
470508
auto srcStrategy = dstStrategy;
471509

472510
using AllocationRecord = umpire::util::AllocationRecord;

src/axom/core/tests/CMakeLists.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,18 @@ if (AXOM_ENABLE_OPENMP AND RAJA_FOUND)
153153
COMMAND core_openmp_tests --gtest_filter=${test_suite}*
154154
NUM_OMP_THREADS ${AXOM_TEST_NUM_OMP_THREADS} )
155155
endforeach()
156+
157+
if (UMPIRE_FOUND)
158+
axom_add_executable(NAME core_copy_openmp_race_test
159+
SOURCES core_copy_openmp_race.cpp
160+
OUTPUT_DIR ${TEST_OUTPUT_DIRECTORY}
161+
DEPENDS_ON ${core_test_depends}
162+
FOLDER axom/core/tests )
163+
164+
axom_add_test( NAME core_copy_openmp_race
165+
COMMAND core_copy_openmp_race_test
166+
NUM_OMP_THREADS ${AXOM_TEST_NUM_OMP_THREADS} )
167+
endif()
156168
endif()
157169

158170
#------------------------------------------------------------------------------
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Copyright (c) Lawrence Livermore National Security, LLC and other
2+
// Axom Project Contributors. See top-level LICENSE and COPYRIGHT
3+
// files for dates and other details.
4+
//
5+
// SPDX-License-Identifier: (BSD-3-Clause)
6+
7+
#include "gtest/gtest.h"
8+
9+
#include "axom/core/Array.hpp"
10+
#include "axom/core/execution/for_all.hpp"
11+
#include "axom/core/memory_management.hpp"
12+
13+
namespace
14+
{
15+
template <typename ExecSpace>
16+
struct CopySlices
17+
{
18+
static void run(int* dst, const int* src, axom::IndexType nslices, axom::IndexType sliceSize)
19+
{
20+
axom::for_all<ExecSpace>(nslices, [=](axom::IndexType slice) {
21+
axom::copy(dst + slice * sliceSize, src + slice * sliceSize, sliceSize * sizeof(int));
22+
});
23+
}
24+
};
25+
} // namespace
26+
27+
TEST(core_copy_openmp_race, first_use_inside_for_all)
28+
{
29+
constexpr axom::IndexType nslices = 128;
30+
constexpr axom::IndexType sliceSize = 64;
31+
constexpr axom::IndexType size = nslices * sliceSize;
32+
33+
axom::Array<int> src(size);
34+
axom::Array<int> dst(size);
35+
36+
for(axom::IndexType i = 0; i < size; ++i)
37+
{
38+
src[i] = static_cast<int>(i);
39+
dst[i] = -1;
40+
}
41+
42+
CopySlices<axom::OMP_EXEC>::run(dst.data(), src.data(), nslices, sliceSize);
43+
44+
for(axom::IndexType i = 0; i < size; ++i)
45+
{
46+
EXPECT_EQ(dst[i], src[i]);
47+
}
48+
}
49+
50+
int main(int argc, char** argv)
51+
{
52+
::testing::InitGoogleTest(&argc, argv);
53+
::testing::FLAGS_gtest_death_test_style = "threadsafe";
54+
return RUN_ALL_TESTS();
55+
}

0 commit comments

Comments
 (0)