From 9bfea8d23325476ea32d593c1db0f4c969cf3a8d Mon Sep 17 00:00:00 2001
From: Alexandr Lyapunov <a.lyapunov@corp.mail.ru>
Date: Thu, 9 Jul 2015 19:10:01 +0300
Subject: [PATCH] fixed gh-731 : now every rtree has it's own matras instance
that is destroyed with rtree, releasing all memory of tree instance
---
src/box/memtx_rtree.cc | 43 ++--------------------------------------
src/lib/salad/rtree.c | 34 ++++++++++++++++++++++---------
src/lib/salad/rtree.h | 23 +++++++++++----------
test/unit/CMakeLists.txt | 4 ++--
test/unit/rtree.cc | 18 +++++++++--------
test/unit/rtree_itr.cc | 42 +++++++++++++++++++++++----------------
6 files changed, 77 insertions(+), 87 deletions(-)
diff --git a/src/box/memtx_rtree.cc b/src/box/memtx_rtree.cc
index 7106aa0f7c..4f5e414951 100644
--- a/src/box/memtx_rtree.cc
+++ b/src/box/memtx_rtree.cc
@@ -34,11 +34,6 @@
#include "fiber.h"
#include "small/small.h"
-/**
- * Single-linked list of free rtree pages
- */
-static void *rtree_free_pages = 0;
-
/* {{{ Utilities. *************************************************/
inline void extract_rectangle(struct rtree_rect *rect,
@@ -122,41 +117,6 @@ index_rtree_iterator_next(struct iterator *i)
/* {{{ MemtxRTree **********************************************************/
-static void *
-rtree_page_alloc()
-{
- ERROR_INJECT(ERRINJ_INDEX_ALLOC, return 0);
- if (!rtree_free_pages) {
- /**
- * No free pages in list - let's allocate new extent, split it
- * into pages and add them to the list.
- */
- char *extent = (char *)memtx_index_extent_alloc();
- if (!extent) {
- panic("%s", "Memory allocation failed in rtree");
- return 0;
- }
- assert(MEMTX_EXTENT_SIZE % RTREE_PAGE_SIZE == 0);
- assert(RTREE_PAGE_SIZE >= sizeof(void *));
- for (size_t i = 0; i < MEMTX_EXTENT_SIZE; i += RTREE_PAGE_SIZE) {
- *(void **)(extent + i) = rtree_free_pages;
- rtree_free_pages = (void *)(extent + i);
- }
- }
- /* Now we surely have a free page in free list */
- void *res = rtree_free_pages;
- rtree_free_pages = *(void **)rtree_free_pages;
- return res;
-}
-
-static void
-rtree_page_free(void *page)
-{
- /* Just add to free list. */
- *(void **)page = rtree_free_pages;
- rtree_free_pages = page;
-}
-
MemtxRTree::~MemtxRTree()
{
// Iterator has to be destroyed prior to tree
@@ -175,7 +135,8 @@ MemtxRTree::MemtxRTree(struct key_def *key_def)
assert(key_def->is_unique == false);
memtx_index_arena_init();
- rtree_init(&tree, rtree_page_alloc, rtree_page_free);
+ rtree_init(&tree, MEMTX_EXTENT_SIZE,
+ memtx_index_extent_alloc, memtx_index_extent_free);
}
size_t
diff --git a/src/lib/salad/rtree.c b/src/lib/salad/rtree.c
index a14f89134e..70a43d410c 100644
--- a/src/lib/salad/rtree.c
+++ b/src/lib/salad/rtree.c
@@ -249,13 +249,23 @@ rtree_always_true(const struct rtree_rect *rt1,
static struct rtree_page *
rtree_alloc_page(struct rtree *tree)
{
- return (struct rtree_page *)tree->page_alloc();
+ if (tree->free_pages) {
+ struct rtree_page *result =
+ (struct rtree_page *)tree->free_pages;
+ tree->free_pages = *(void **)tree->free_pages;
+ return result;
+ } else {
+ uint32_t unused_id;
+ return (struct rtree_page *)
+ matras_alloc(&tree->mtab, &unused_id);
+ }
}
static void
rtree_free_page(struct rtree *tree, struct rtree_page *page)
{
- tree->page_free(page);
+ *(void **)page = tree->free_pages;
+ tree->free_pages = (void *)page;
}
static void
@@ -558,7 +568,7 @@ rtree_page_purge(struct rtree *tree, struct rtree_page *page, int level)
for (int i = 0; i < page->n; i++)
rtree_page_purge(tree, page->b[i].data.page, level);
}
- tree->page_free(page);
+ rtree_free_page(tree, page);
}
/*------------------------------------------------------------------------- */
@@ -624,7 +634,8 @@ rtree_iterator_destroy(struct rtree_iterator *itr)
struct rtree_neighbor_page *curr, *next;
for (curr = itr->page_list; curr != NULL; curr = next) {
next = curr->next;
- itr->tree->page_free(curr);
+ rtree_free_page((struct rtree *) itr->tree,
+ (struct rtree_page *) curr);
}
itr->page_list = NULL;
itr->page_pos = RTREE_NEIGHBORS_IN_PAGE;
@@ -648,7 +659,8 @@ rtree_iterator_allocate_neighbour(struct rtree_iterator *itr)
{
if (itr->page_pos >= RTREE_NEIGHBORS_IN_PAGE) {
struct rtree_neighbor_page *new_page =
- (struct rtree_neighbor_page *)itr->tree->page_alloc();
+ (struct rtree_neighbor_page *)
+ rtree_alloc_page((struct rtree*)itr->tree);
new_page->next = itr->page_list;
itr->page_list = new_page;
itr->page_pos = 0;
@@ -683,6 +695,7 @@ rtree_iterator_free_neighbor(struct rtree_iterator *itr,
void
rtree_iterator_init(struct rtree_iterator *itr)
{
+ itr->tree = 0;
itr->neigh_list = NULL;
itr->neigh_free_list = NULL;
itr->page_list = NULL;
@@ -764,22 +777,24 @@ rtree_iterator_next(struct rtree_iterator *itr)
/*------------------------------------------------------------------------- */
void
-rtree_init(struct rtree *tree,
- rtree_page_alloc_t page_alloc, rtree_page_free_t page_free)
+rtree_init(struct rtree *tree, uint32_t extent_size,
+ rtree_extent_alloc_t extent_alloc, rtree_extent_free_t extent_free)
{
tree->n_records = 0;
tree->height = 0;
tree->root = NULL;
tree->version = 0;
tree->n_pages = 0;
- tree->page_alloc = page_alloc;
- tree->page_free = page_free;
+ matras_create(&tree->mtab, extent_size, RTREE_PAGE_SIZE,
+ extent_alloc, extent_free);
+ tree->free_pages = 0;
}
void
rtree_destroy(struct rtree *tree)
{
rtree_purge(tree);
+ matras_destroy(&tree->mtab);
}
void
@@ -859,6 +874,7 @@ rtree_search(const struct rtree *tree, const struct rtree_rect *rect,
enum spatial_search_op op, struct rtree_iterator *itr)
{
rtree_iterator_reset(itr);
+ assert(itr->tree == 0 || itr->tree == tree);
itr->tree = tree;
itr->version = tree->version;
itr->rect = *rect;
diff --git a/src/lib/salad/rtree.h b/src/lib/salad/rtree.h
index f3e6208500..4d1f4c2e28 100644
--- a/src/lib/salad/rtree.h
+++ b/src/lib/salad/rtree.h
@@ -30,6 +30,7 @@
*/
#include <stddef.h>
#include <stdbool.h>
+#include "small/matras.h"
/**
* In-memory Guttman's R-tree
@@ -56,6 +57,7 @@ enum {
/**
* R-Tree uses linear search for elements on a page,
* so a larger page size can hurt performance.
+ * must be power of 2
*/
RTREE_PAGE_SIZE = 1024
};
@@ -88,8 +90,8 @@ enum spatial_search_op
};
/* pointers to page allocation and deallocations functions */
-typedef void *(*rtree_page_alloc_t)();
-typedef void (*rtree_page_free_t)(void *);
+typedef void *(*rtree_extent_alloc_t)();
+typedef void (*rtree_extent_free_t)(void *);
/* A point in RTREE_DIMENSION space */
struct rtree_point
@@ -124,10 +126,10 @@ struct rtree
unsigned version;
/* Number of allocated (used) pages */
unsigned n_pages;
- /* Function for allocation new pages */
- rtree_page_alloc_t page_alloc;
- /* Function for deallocation new pages */
- rtree_page_free_t page_free;
+ /* Matras for allocating new page */
+ struct matras mtab;
+ /* List of free pages */
+ void *free_pages;
};
/* Struct for iteration and retrieving rtree values */
@@ -196,12 +198,13 @@ rtree_set2d(struct rtree_rect *rect,
/**
* @brief Initialize a tree
* @param tree - pointer to a tree
- * @param page_alloc - page allocation function
- * @param page_free - page deallocation function
+ * @param extent_size - size of extents allocated by extent_alloc (see next)
+ * @param extent_alloc - extent allocation function
+ * @param extent_free - extent deallocation function
*/
void
-rtree_init(struct rtree *tree,
- rtree_page_alloc_t page_alloc, rtree_page_free_t page_free);
+rtree_init(struct rtree *tree, uint32_t extent_size,
+ rtree_extent_alloc_t extent_alloc, rtree_extent_free_t extent_free);
/**
* @brief Destroy a tree
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index 485c94971a..9582135223 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -44,9 +44,9 @@ target_link_libraries(bps_tree.test small)
add_executable(bps_tree_itr.test bps_tree_itr.cc ${CMAKE_SOURCE_DIR}/third_party/qsort_arg.c)
target_link_libraries(bps_tree_itr.test small)
add_executable(rtree.test rtree.cc ${CMAKE_SOURCE_DIR}/src/lib/salad/rtree.c)
-target_link_libraries(rtree.test)
+target_link_libraries(rtree.test small)
add_executable(rtree_itr.test rtree_itr.cc ${CMAKE_SOURCE_DIR}/src/lib/salad/rtree.c)
-target_link_libraries(rtree_itr.test)
+target_link_libraries(rtree_itr.test small)
add_executable(matras.test matras.cc)
target_link_libraries(matras.test small)
add_executable(light.test light.cc)
diff --git a/test/unit/rtree.cc b/test/unit/rtree.cc
index 9e4960c862..e8836deb9d 100644
--- a/test/unit/rtree.cc
+++ b/test/unit/rtree.cc
@@ -9,15 +9,17 @@
static int page_count = 0;
+const uint32_t extent_size = RTREE_PAGE_SIZE * 8;
+
static void *
-page_alloc()
+extent_alloc()
{
page_count++;
- return malloc(RTREE_PAGE_SIZE);
+ return malloc(extent_size);
}
static void
-page_free(void *page)
+extent_free(void *page)
{
page_count--;
free(page);
@@ -34,7 +36,7 @@ simple_check()
header();
struct rtree tree;
- rtree_init(&tree, page_alloc, page_free);
+ rtree_init(&tree, extent_size, extent_alloc, extent_free);
printf("Insert 1..X, remove 1..X\n");
for (size_t i = 1; i <= rounds; i++) {
@@ -241,8 +243,6 @@ neighbor_test()
header();
const int test_count = 1000;
- struct rtree_iterator iterator;
- rtree_iterator_init(&iterator);
struct rtree_rect arr[test_count];
static struct rtree_rect basis;
@@ -255,10 +255,12 @@ neighbor_test()
for (size_t i = 0; i <= test_count; i++) {
struct rtree tree;
- rtree_init(&tree, page_alloc, page_free);
+ rtree_init(&tree, extent_size, extent_alloc, extent_free);
rtree_test_build(&tree, arr, i);
+ struct rtree_iterator iterator;
+ rtree_iterator_init(&iterator);
if (!rtree_search(&tree, &basis, SOP_NEIGHBOR, &iterator) && i != 0) {
fail("search is successful", "true");
}
@@ -269,10 +271,10 @@ neighbor_test()
fail("wrong search result", "true");
}
}
+ rtree_iterator_destroy(&iterator);
rtree_destroy(&tree);
}
- rtree_iterator_destroy(&iterator);
footer();
}
diff --git a/test/unit/rtree_itr.cc b/test/unit/rtree_itr.cc
index fae7c43110..08bc959aec 100644
--- a/test/unit/rtree_itr.cc
+++ b/test/unit/rtree_itr.cc
@@ -6,19 +6,21 @@
#include "unit.h"
#include "salad/rtree.h"
-static int page_count = 0;
+static int extent_count = 0;
+
+const uint32_t extent_size = RTREE_PAGE_SIZE * 8;
static void *
-page_alloc()
+extent_alloc()
{
- page_count++;
- return malloc(RTREE_PAGE_SIZE);
+ extent_count++;
+ return malloc(extent_size);
}
static void
-page_free(void *page)
+extent_free(void *page)
{
- page_count--;
+ extent_count--;
free(page);
}
@@ -28,7 +30,7 @@ itr_check()
header();
struct rtree tree;
- rtree_init(&tree, page_alloc, page_free);
+ rtree_init(&tree, extent_size, extent_alloc, extent_free);
/* Filling tree */
const size_t count1 = 10000;
@@ -182,8 +184,8 @@ itr_check()
}
rtree_purge(&tree);
- rtree_destroy(&tree);
rtree_iterator_destroy(&iterator);
+ rtree_destroy(&tree);
footer();
}
@@ -197,9 +199,6 @@ itr_invalidate_check()
const size_t max_delete_count = 100;
const size_t max_insert_count = 200;
const size_t attempt_count = 100;
- struct rtree_iterator iterators[test_size];
- for (size_t i = 0; i < test_size; i++)
- rtree_iterator_init(iterators + i);
struct rtree_rect rect;
@@ -212,7 +211,10 @@ itr_invalidate_check()
del_cnt = test_size - del_pos;
}
struct rtree tree;
- rtree_init(&tree, page_alloc, page_free);
+ rtree_init(&tree, extent_size, extent_alloc, extent_free);
+ struct rtree_iterator iterators[test_size];
+ for (size_t i = 0; i < test_size; i++)
+ rtree_iterator_init(iterators + i);
for (size_t i = 0; i < test_size; i++) {
rtree_set2d(&rect, i, i, i, i);
@@ -240,6 +242,9 @@ itr_invalidate_check()
fail("Iterator was not invalidated (18)", "true");
}
}
+
+ for (size_t i = 0; i < test_size; i++)
+ rtree_iterator_destroy(iterators + i);
rtree_destroy(&tree);
}
@@ -250,7 +255,10 @@ itr_invalidate_check()
size_t ins_cnt = rand() % max_insert_count + 1;
struct rtree tree;
- rtree_init(&tree, page_alloc, page_free);
+ rtree_init(&tree, extent_size, extent_alloc, extent_free);
+ struct rtree_iterator iterators[test_size];
+ for (size_t i = 0; i < test_size; i++)
+ rtree_iterator_init(iterators + i);
for (size_t i = 0; i < test_size; i++) {
rtree_set2d(&rect, i, i, i, i);
@@ -276,12 +284,12 @@ itr_invalidate_check()
fail("Iterator was not invalidated (22)", "true");
}
}
+
+ for (size_t i = 0; i < test_size; i++)
+ rtree_iterator_destroy(iterators + i);
rtree_destroy(&tree);
}
- for (size_t i = 0; i < test_size; i++)
- rtree_iterator_destroy(iterators + i);
-
footer();
}
@@ -290,7 +298,7 @@ main(void)
{
itr_check();
itr_invalidate_check();
- if (page_count != 0) {
+ if (extent_count != 0) {
fail("memory leak!", "false");
}
}
--
GitLab