Skip to content

Commit de9ee6e

Browse files
ldorauldorau
committed
FIX merging and spliting
1 parent 5ef3c0a commit de9ee6e

File tree

2 files changed

+119
-57
lines changed

2 files changed

+119
-57
lines changed

‎src/provider/provider_coarse.c‎

Lines changed: 34 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,18 @@ static block_t *get_block_next(ravl_node_t *node) {
158158
}
159159
#endif /* NDEBUG */
160160

161+
static ravl_node_t *get_origin_node(struct ravl *upstream_blocks,
162+
block_t *block) {
163+
ravl_data_t rdata = {(uintptr_t)block->data, NULL};
164+
ravl_node_t *ravl_origin =
165+
ravl_find(upstream_blocks, &rdata, RAVL_PREDICATE_LESS_EQUAL);
166+
assert(ravl_origin);
167+
168+
block_t *origin = get_node_block(ravl_origin);
169+
assert(IS_ORIGIN_OF_BLOCK(origin, block));
170+
return ravl_origin;
171+
}
172+
161173
static bool is_same_origin(struct ravl *upstream_blocks, block_t *block1,
162174
block_t *block2) {
163175
ravl_data_t rdata1 = {(uintptr_t)block1->data, NULL};
@@ -456,6 +468,11 @@ static block_t *free_blocks_rm_node(struct ravl *free_blocks,
456468
return block;
457469
}
458470

471+
static umf_result_t
472+
upstream_block_merge(coarse_memory_provider_t *coarse_provider,
473+
ravl_node_t *node1, ravl_node_t *node2,
474+
ravl_node_t **merged_node);
475+
459476
// user_block_merge - merge two blocks from one of two lists of user blocks: all_blocks or free_blocks
460477
static umf_result_t user_block_merge(coarse_memory_provider_t *coarse_provider,
461478
ravl_node_t *node1, ravl_node_t *node2,
@@ -478,13 +495,27 @@ static umf_result_t user_block_merge(coarse_memory_provider_t *coarse_provider,
478495

479496
bool same_used = ((block1->used == used) && (block2->used == used));
480497
bool contignous_data = (block1->data + block1->size == block2->data);
481-
bool same_origin = is_same_origin(upstream_blocks, block1, block2);
482498

483499
// check if blocks can be merged
484-
if (!same_used || !contignous_data || !same_origin) {
500+
if (!same_used || !contignous_data) {
485501
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
486502
}
487503

504+
// check if blocks have the same origin
505+
ravl_node_t *origin1 = get_origin_node(upstream_blocks, block1);
506+
if (!IS_ORIGIN_OF_BLOCK((get_node_block(origin1)), block2)) {
507+
// origins are different - they have to be merged
508+
ravl_node_t *origin2 = get_origin_node(upstream_blocks, block2);
509+
510+
// merge origins
511+
ravl_node_t *merged_node = NULL;
512+
umf_result_t umf_result = upstream_block_merge(coarse_provider, origin1,
513+
origin2, &merged_node);
514+
if (umf_result != UMF_RESULT_SUCCESS) {
515+
return umf_result;
516+
}
517+
}
518+
488519
if (block1->free_list_ptr) {
489520
free_blocks_rm_node(free_blocks, block1->free_list_ptr);
490521
block1->free_list_ptr = NULL;
@@ -602,56 +633,6 @@ upstream_block_merge(coarse_memory_provider_t *coarse_provider,
602633
return UMF_RESULT_SUCCESS;
603634
}
604635

605-
// upstream_block_merge_with_prev - merge the given upstream block
606-
// with the previous one if both have continuous data.
607-
// Remove the merged block from the tree of upstream blocks.
608-
static ravl_node_t *
609-
upstream_block_merge_with_prev(coarse_memory_provider_t *coarse_provider,
610-
ravl_node_t *node) {
611-
assert(node);
612-
613-
ravl_node_t *node_prev = get_node_prev(node);
614-
if (!node_prev) {
615-
return node;
616-
}
617-
618-
ravl_node_t *merged_node = NULL;
619-
umf_result_t umf_result =
620-
upstream_block_merge(coarse_provider, node_prev, node, &merged_node);
621-
if (umf_result != UMF_RESULT_SUCCESS) {
622-
return node;
623-
}
624-
625-
assert(merged_node != NULL);
626-
627-
return merged_node;
628-
}
629-
630-
// upstream_block_merge_with_next - merge the given upstream block
631-
// with the next one if both have continuous data.
632-
// Remove the merged block from the tree of upstream blocks.
633-
static ravl_node_t *
634-
upstream_block_merge_with_next(coarse_memory_provider_t *coarse_provider,
635-
ravl_node_t *node) {
636-
assert(node);
637-
638-
ravl_node_t *node_next = get_node_next(node);
639-
if (!node_next) {
640-
return node;
641-
}
642-
643-
ravl_node_t *merged_node = NULL;
644-
umf_result_t umf_result =
645-
upstream_block_merge(coarse_provider, node, node_next, &merged_node);
646-
if (umf_result != UMF_RESULT_SUCCESS) {
647-
return node;
648-
}
649-
650-
assert(merged_node != NULL);
651-
652-
return merged_node;
653-
}
654-
655636
#ifndef NDEBUG // begin of DEBUG code
656637

657638
typedef struct debug_cb_args_t {
@@ -812,10 +793,6 @@ coarse_add_upstream_block(coarse_memory_provider_t *coarse_provider, void *addr,
812793
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
813794
}
814795

815-
// check if the new upstream block can be merged with its neighbours
816-
alloc_node = upstream_block_merge_with_prev(coarse_provider, alloc_node);
817-
alloc_node = upstream_block_merge_with_next(coarse_provider, alloc_node);
818-
819796
new_block->used = true;
820797
coarse_provider->alloc_size += size;
821798
coarse_provider->used_size += size;
@@ -1645,7 +1622,7 @@ static umf_result_t coarse_memory_provider_allocation_merge(void *provider,
16451622
}
16461623

16471624
if ((uintptr_t)highPtr != ((uintptr_t)lowPtr + low_block->size)) {
1648-
LOG_ERR("given pointers cannot be merged");
1625+
LOG_ERR("given allocations are not adjacent");
16491626
umf_result = UMF_RESULT_ERROR_INVALID_ARGUMENT;
16501627
goto err_mutex_unlock;
16511628
}

‎test/provider_coarse.cpp‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "provider.hpp"
1111

1212
#include <umf/providers/provider_coarse.h>
13+
#include <umf/providers/provider_file_memory.h>
1314

1415
using umf_test::KB;
1516
using umf_test::MB;
@@ -21,6 +22,8 @@ using umf_test::test;
2122
#define BASE_NAME "coarse"
2223
#define COARSE_NAME BASE_NAME " (" UPSTREAM_NAME ")"
2324

25+
#define FILE_PATH ((char *)"tmp_file")
26+
2427
umf_memory_provider_ops_t UMF_MALLOC_MEMORY_PROVIDER_OPS =
2528
umf::providerMakeCOps<umf_test::provider_ba_global, void>();
2629

@@ -340,6 +343,88 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseProvider_wrong_params_5) {
340343
ASSERT_EQ(coarse_memory_provider, nullptr);
341344
}
342345

346+
#ifndef _WIN32
347+
TEST_P(CoarseWithMemoryStrategyTest, coarseProvider_merge_upstream) {
348+
umf_memory_provider_handle_t file_memory_provider;
349+
umf_result_t umf_result;
350+
351+
umf_file_memory_provider_params_t file_params =
352+
umfFileMemoryProviderParamsDefault(FILE_PATH);
353+
file_params.visibility = UMF_MEM_MAP_SHARED;
354+
355+
umf_result = umfMemoryProviderCreate(umfFileMemoryProviderOps(),
356+
&file_params, &file_memory_provider);
357+
ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
358+
ASSERT_NE(file_memory_provider, nullptr);
359+
360+
const size_t init_buffer_size = 20 * MB;
361+
362+
coarse_memory_provider_params_t coarse_memory_provider_params;
363+
// make sure there are no undefined members - prevent a UB
364+
memset(&coarse_memory_provider_params, 0,
365+
sizeof(coarse_memory_provider_params));
366+
coarse_memory_provider_params.upstream_memory_provider =
367+
file_memory_provider;
368+
coarse_memory_provider_params.immediate_init_from_upstream = true;
369+
coarse_memory_provider_params.init_buffer = NULL;
370+
coarse_memory_provider_params.init_buffer_size = init_buffer_size;
371+
372+
umf_memory_provider_handle_t coarse_memory_provider;
373+
umf_result = umfMemoryProviderCreate(umfCoarseMemoryProviderOps(),
374+
&coarse_memory_provider_params,
375+
&coarse_memory_provider);
376+
ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
377+
ASSERT_NE(coarse_memory_provider, nullptr);
378+
379+
umf_memory_provider_handle_t cp = coarse_memory_provider;
380+
char *ptr1 = nullptr;
381+
char *ptr2 = nullptr;
382+
383+
ASSERT_EQ(GetStats(cp).used_size, 0 * MB);
384+
ASSERT_EQ(GetStats(cp).alloc_size, init_buffer_size);
385+
ASSERT_EQ(GetStats(cp).num_all_blocks, 1);
386+
387+
/* test umfMemoryProviderAllocationMerge */
388+
umf_result =
389+
umfMemoryProviderAlloc(cp, init_buffer_size, 0, (void **)&ptr1);
390+
ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
391+
ASSERT_NE(ptr1, nullptr);
392+
ASSERT_EQ(GetStats(cp).used_size, init_buffer_size);
393+
ASSERT_EQ(GetStats(cp).alloc_size, init_buffer_size);
394+
ASSERT_EQ(GetStats(cp).num_all_blocks, 1);
395+
396+
umf_result =
397+
umfMemoryProviderAlloc(cp, init_buffer_size, 0, (void **)&ptr2);
398+
ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
399+
ASSERT_NE(ptr2, nullptr);
400+
ASSERT_EQ(GetStats(cp).used_size, 2 * init_buffer_size);
401+
ASSERT_EQ(GetStats(cp).alloc_size, 2 * init_buffer_size);
402+
ASSERT_EQ(GetStats(cp).num_all_blocks, 2);
403+
404+
if ((uintptr_t)ptr1 < (uintptr_t)ptr2) {
405+
umf_result = umfMemoryProviderAllocationMerge(cp, ptr1, ptr2,
406+
2 * init_buffer_size);
407+
} else {
408+
umf_result = umfMemoryProviderAllocationMerge(cp, ptr2, ptr1,
409+
2 * init_buffer_size);
410+
ptr1 = ptr2;
411+
}
412+
ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
413+
ASSERT_EQ(GetStats(cp).used_size, 2 * init_buffer_size);
414+
ASSERT_EQ(GetStats(cp).alloc_size, 2 * init_buffer_size);
415+
ASSERT_EQ(GetStats(cp).num_all_blocks, 1);
416+
417+
umf_result = umfMemoryProviderFree(cp, ptr1, 2 * init_buffer_size);
418+
ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
419+
ASSERT_EQ(GetStats(cp).used_size, 0);
420+
ASSERT_EQ(GetStats(cp).alloc_size, 2 * init_buffer_size);
421+
ASSERT_EQ(GetStats(cp).num_all_blocks, 1);
422+
423+
umfMemoryProviderDestroy(coarse_memory_provider);
424+
umfMemoryProviderDestroy(file_memory_provider);
425+
}
426+
#endif /* _WIN32 */
427+
343428
TEST_P(CoarseWithMemoryStrategyTest, coarseProvider_split_merge) {
344429
umf_memory_provider_handle_t malloc_memory_provider;
345430
umf_result_t umf_result;

0 commit comments

Comments
 (0)