Merge pull request #690 from vinser52/svinogra_ipc_tests

Refactor IPC tests and enable them for OS provider

Author: lukaszstolarczuk

src/ipc.c

@@ -51,6 +51,11 @@ umf_result_t umfPoolGetIPCHandleSize(umf_memory_pool_handle_t hPool,
 
 umf_result_t umfGetIPCHandle(const void *ptr, umf_ipc_handle_t *umfIPCHandle,
                              size_t *size) {
+    if (ptr == NULL || umfIPCHandle == NULL || size == NULL) {
+        LOG_ERR("invalid argument.");
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     size_t ipcHandleSize = 0;
     umf_alloc_info_t allocInfo;
     umf_result_t ret = umfMemoryTrackerGetAllocInfo(ptr, &allocInfo);

test/CMakeLists.txt

@@ -186,6 +186,12 @@ if(LINUX AND (NOT UMF_DISABLE_HWLOC)) # OS-specific functions are implemented
         NAME provider_os_memory
         SRCS provider_os_memory.cpp
         LIBS ${UMF_UTILS_FOR_TEST})
+    if(UMF_BUILD_LIBUMF_POOL_DISJOINT)
+        target_compile_definitions(umf_test-provider_os_memory
+                                   PRIVATE UMF_POOL_DISJOINT_ENABLED=1)
+        target_link_libraries(umf_test-provider_os_memory PRIVATE disjoint_pool)
+    endif()
+
     add_umf_test(
         NAME provider_os_memory_multiple_numa_nodes
         SRCS provider_os_memory_multiple_numa_nodes.cpp

test/ipcFixtures.hpp

@@ -52,11 +52,8 @@ using ipcTestParams =
 
 struct umfIpcTest : umf_test::test,
                     ::testing::WithParamInterface<ipcTestParams> {
-    umfIpcTest() : pool(nullptr, nullptr) {}
-    void SetUp() override {
-        test::SetUp();
-        this->pool = makePool();
-    }
+    umfIpcTest() {}
+    void SetUp() override { test::SetUp(); }
 
     void TearDown() override { test::TearDown(); }
 
@@ -74,7 +71,9 @@ struct umfIpcTest : umf_test::test,
 
         auto trace = [](void *trace_context, const char *name) {
             stats_type *stat = static_cast<stats_type *>(trace_context);
-            if (std::strcmp(name, "get_ipc_handle") == 0) {
+            if (std::strcmp(name, "alloc") == 0) {
+                ++stat->allocCount;
+            } else if (std::strcmp(name, "get_ipc_handle") == 0) {
                 ++stat->getCount;
             } else if (std::strcmp(name, "put_ipc_handle") == 0) {
                 ++stat->putCount;
@@ -98,30 +97,60 @@ struct umfIpcTest : umf_test::test,
     }
 
     struct stats_type {
+        std::atomic<size_t> allocCount;
         std::atomic<size_t> getCount;
         std::atomic<size_t> putCount;
         std::atomic<size_t> openCount;
         std::atomic<size_t> closeCount;
 
-        stats_type() : getCount(0), putCount(0), openCount(0), closeCount(0) {}
+        stats_type()
+            : allocCount(0), getCount(0), putCount(0), openCount(0),
+              closeCount(0) {}
     };
 
-    umf::pool_unique_handle_t pool;
     static constexpr int NTHREADS = 10;
     stats_type stat;
     MemoryAccessor *memAccessor = nullptr;
 };
 
 TEST_P(umfIpcTest, GetIPCHandleSize) {
     size_t size = 0;
+    umf::pool_unique_handle_t pool = makePool();
+
     umf_result_t ret = umfPoolGetIPCHandleSize(pool.get(), &size);
     EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
     EXPECT_GT(size, 0);
 }
 
+TEST_P(umfIpcTest, GetIPCHandleInvalidArgs) {
+    constexpr size_t SIZE = 100;
+    umf_ipc_handle_t ipcHandle = nullptr;
+    size_t handleSize = 0;
+    umf_result_t ret = umfGetIPCHandle(nullptr, &ipcHandle, &handleSize);
+    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    void *ptr = (void *)0xBAD;
+    ret = umfGetIPCHandle(ptr, &ipcHandle, &handleSize);
+    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    umf::pool_unique_handle_t pool = makePool();
+    ptr = umfPoolMalloc(pool.get(), SIZE);
+    EXPECT_NE(ptr, nullptr);
+
+    ret = umfGetIPCHandle(ptr, nullptr, &handleSize);
+    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfGetIPCHandle(ptr, &ipcHandle, nullptr);
+    EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfFree(ptr);
+    EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
+}
+
 TEST_P(umfIpcTest, BasicFlow) {
     constexpr size_t SIZE = 100;
     std::vector<int> expected_data(SIZE);
+    umf::pool_unique_handle_t pool = makePool();
     int *ptr = (int *)umfPoolMalloc(pool.get(), SIZE * sizeof(int));
     EXPECT_NE(ptr, nullptr);
 
@@ -172,6 +201,7 @@ TEST_P(umfIpcTest, BasicFlow) {
     ret = umfPoolFree(pool.get(), ptr);
     EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
 
+    pool.reset(nullptr);
     EXPECT_EQ(stat.getCount, 1);
     EXPECT_EQ(stat.putCount, stat.getCount);
     // TODO: enale check below once cache for open IPC handles is implemented
@@ -183,6 +213,8 @@ TEST_P(umfIpcTest, ConcurrentGetPutHandles) {
     std::vector<void *> ptrs;
     constexpr size_t ALLOC_SIZE = 100;
     constexpr size_t NUM_POINTERS = 100;
+    umf::pool_unique_handle_t pool = makePool();
+
     for (size_t i = 0; i < NUM_POINTERS; ++i) {
         void *ptr = umfPoolMalloc(pool.get(), ALLOC_SIZE);
         EXPECT_NE(ptr, nullptr);
@@ -221,15 +253,16 @@ TEST_P(umfIpcTest, ConcurrentGetPutHandles) {
         EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
     }
 
-    EXPECT_GE(stat.getCount, NUM_POINTERS);
-    EXPECT_LE(stat.getCount, NUM_POINTERS * NTHREADS);
+    pool.reset(nullptr);
     EXPECT_EQ(stat.putCount, stat.getCount);
 }
 
 TEST_P(umfIpcTest, ConcurrentOpenCloseHandles) {
     std::vector<void *> ptrs;
     constexpr size_t ALLOC_SIZE = 100;
     constexpr size_t NUM_POINTERS = 100;
+    umf::pool_unique_handle_t pool = makePool();
+
     for (size_t i = 0; i < NUM_POINTERS; ++i) {
         void *ptr = umfPoolMalloc(pool.get(), ALLOC_SIZE);
         EXPECT_NE(ptr, nullptr);
@@ -249,8 +282,8 @@ TEST_P(umfIpcTest, ConcurrentOpenCloseHandles) {
 
     umf_test::syncthreads_barrier syncthreads(NTHREADS);
 
-    auto openHandlesFn = [this, &ipcHandles, &openedIpcHandles,
-                          &syncthreads](size_t tid) {
+    auto openHandlesFn = [this, &ipcHandles, &openedIpcHandles, &syncthreads,
+                          &pool](size_t tid) {
         syncthreads();
         for (auto ipcHandle : ipcHandles) {
             void *ptr;
@@ -282,6 +315,11 @@ TEST_P(umfIpcTest, ConcurrentOpenCloseHandles) {
         EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
     }
 
+    pool.reset(nullptr);
+    EXPECT_EQ(stat.getCount, stat.allocCount);
+    EXPECT_EQ(stat.putCount, stat.getCount);
+    // TODO: enale check below once cache for open IPC handles is implemented
+    // EXPECT_EQ(stat.openCount, stat.allocCount);
     EXPECT_EQ(stat.openCount, stat.closeCount);
 }
 

test/provider_os_memory.cpp

@@ -5,8 +5,10 @@
 #include "base.hpp"
 
 #include "cpp_helpers.hpp"
+#include "ipcFixtures.hpp"
 
 #include <umf/memory_provider.h>
+#include <umf/pools/pool_disjoint.h>
 #include <umf/providers/provider_os_memory.h>
 
 using umf_test::test;
@@ -327,3 +329,34 @@ TEST_P(umfProviderTest, purge_force_INVALID_POINTER) {
     verify_last_native_error(provider.get(),
                              UMF_OS_RESULT_ERROR_PURGE_FORCE_FAILED);
 }
+
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfIpcTest);
+
+umf_os_memory_provider_params_t osMemoryProviderParamsShared() {
+    auto params = umfOsMemoryProviderParamsDefault();
+    params.visibility = UMF_MEM_MAP_SHARED;
+    return params;
+}
+auto os_params = osMemoryProviderParamsShared();
+
+HostMemoryAccessor hostAccessor;
+
+umf_disjoint_pool_params_t disjointPoolParams() {
+    umf_disjoint_pool_params_t params = umfDisjointPoolParamsDefault();
+    params.SlabMinSize = 4096;
+    params.MaxPoolableSize = 4096;
+    params.Capacity = 4;
+    params.MinBucketSize = 64;
+    return params;
+}
+umf_disjoint_pool_params_t disjointParams = disjointPoolParams();
+
+static std::vector<ipcTestParams> ipcTestParamsList = {
+#if (defined UMF_POOL_DISJOINT_ENABLED)
+    {umfDisjointPoolOps(), &disjointParams, umfOsMemoryProviderOps(),
+     &os_params, &hostAccessor},
+#endif
+};
+
+INSTANTIATE_TEST_SUITE_P(osProviderTest, umfIpcTest,
+                         ::testing::ValuesIn(ipcTestParamsList));

test/test_valgrind.sh

@@ -91,6 +91,9 @@ for test in $(ls -1 umf_test-*); do
 	umf_test-memspace_host_all)
 		FILTER='--gtest_filter="-*allocsSpreadAcrossAllNumaNodes"'
 		;;
+	umf_test-provider_os_memory)
+		FILTER='--gtest_filter="-osProviderTest/umfIpcTest*"'
+		;;
 	umf_test-provider_os_memory_config)
 		FILTER='--gtest_filter="-*protection_flag_none:*protection_flag_read:*providerConfigTestNumaMode*"'
 		;;