Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: vdavydov.dev@gmail.com
Subject: [PATCH 2/3] salad: do not touch struct heap_node.pos in user's code
Date: Fri, 22 Feb 2019 14:38:59 +0300	[thread overview]
Message-ID: <36f1e53ccc18d49524061ddc442cf6cb80e3ce4e.1550835301.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1550835301.git.v.shpilevoy@tarantool.org>
In-Reply-To: <cover.1550835301.git.v.shpilevoy@tarantool.org>

The only goal of reading and writing heap_node.pos was checking
if a node is now in a heap, or not. This commit encapsulates this
logic into a couple of functions.
---
 src/box/vy_lsm.c       | 12 ++++++------
 src/box/vy_range.c     |  2 +-
 src/box/vy_range.h     |  2 +-
 src/box/vy_scheduler.c | 23 ++++++++++-------------
 src/lib/salad/heap.h   | 23 ++++++++++++++++++++---
 5 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 57f5a7a29..07da145bd 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -196,8 +196,8 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 		vy_lsm_ref(pk);
 	lsm->mem_format = format;
 	tuple_format_ref(lsm->mem_format);
-	lsm->in_dump.pos = UINT32_MAX;
-	lsm->in_compaction.pos = UINT32_MAX;
+	heap_node_create(&lsm->in_dump);
+	heap_node_create(&lsm->in_compaction);
 	lsm->space_id = index_def->space_id;
 	lsm->index_id = index_def->iid;
 	lsm->group_id = group_id;
@@ -240,8 +240,8 @@ void
 vy_lsm_delete(struct vy_lsm *lsm)
 {
 	assert(lsm->refs == 0);
-	assert(lsm->in_dump.pos == UINT32_MAX);
-	assert(lsm->in_compaction.pos == UINT32_MAX);
+	assert(heap_node_is_stray(&lsm->in_dump));
+	assert(heap_node_is_stray(&lsm->in_compaction));
 	assert(vy_lsm_read_set_empty(&lsm->read_set));
 	assert(lsm->env->lsm_count > 0);
 
@@ -764,7 +764,7 @@ vy_lsm_remove_run(struct vy_lsm *lsm, struct vy_run *run)
 void
 vy_lsm_add_range(struct vy_lsm *lsm, struct vy_range *range)
 {
-	assert(range->heap_node.pos == UINT32_MAX);
+	assert(heap_node_is_stray(&range->heap_node));
 	vy_range_heap_insert(&lsm->range_heap, range);
 	vy_range_tree_insert(&lsm->range_tree, range);
 	lsm->range_count++;
@@ -773,7 +773,7 @@ vy_lsm_add_range(struct vy_lsm *lsm, struct vy_range *range)
 void
 vy_lsm_remove_range(struct vy_lsm *lsm, struct vy_range *range)
 {
-	assert(range->heap_node.pos != UINT32_MAX);
+	assert(! heap_node_is_stray(&range->heap_node));
 	vy_range_heap_delete(&lsm->range_heap, range);
 	vy_range_tree_remove(&lsm->range_tree, range);
 	lsm->range_count--;
diff --git a/src/box/vy_range.c b/src/box/vy_range.c
index d9afcfff3..3491784a4 100644
--- a/src/box/vy_range.c
+++ b/src/box/vy_range.c
@@ -197,7 +197,7 @@ vy_range_new(int64_t id, struct tuple *begin, struct tuple *end,
 	}
 	range->cmp_def = cmp_def;
 	rlist_create(&range->slices);
-	range->heap_node.pos = UINT32_MAX;
+	heap_node_create(&range->heap_node);
 	return range;
 }
 
diff --git a/src/box/vy_range.h b/src/box/vy_range.h
index facd2ac01..91f2682c8 100644
--- a/src/box/vy_range.h
+++ b/src/box/vy_range.h
@@ -156,7 +156,7 @@ vy_range_heap_less(struct vy_range *r1, struct vy_range *r2)
 static inline bool
 vy_range_is_scheduled(struct vy_range *range)
 {
-	return range->heap_node.pos == UINT32_MAX;
+	return heap_node_is_stray(&range->heap_node);
 }
 
 /**
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 073c2c7cf..92d407808 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -514,8 +514,8 @@ void
 vy_scheduler_add_lsm(struct vy_scheduler *scheduler, struct vy_lsm *lsm)
 {
 	assert(!lsm->is_dropped);
-	assert(lsm->in_dump.pos == UINT32_MAX);
-	assert(lsm->in_compaction.pos == UINT32_MAX);
+	assert(heap_node_is_stray(&lsm->in_dump));
+	assert(heap_node_is_stray(&lsm->in_compaction));
 	vy_dump_heap_insert(&scheduler->dump_heap, lsm);
 	vy_compaction_heap_insert(&scheduler->compaction_heap, lsm);
 }
@@ -524,12 +524,10 @@ void
 vy_scheduler_remove_lsm(struct vy_scheduler *scheduler, struct vy_lsm *lsm)
 {
 	assert(!lsm->is_dropped);
-	assert(lsm->in_dump.pos != UINT32_MAX);
-	assert(lsm->in_compaction.pos != UINT32_MAX);
+	assert(! heap_node_is_stray(&lsm->in_dump));
+	assert(! heap_node_is_stray(&lsm->in_compaction));
 	vy_dump_heap_delete(&scheduler->dump_heap, lsm);
 	vy_compaction_heap_delete(&scheduler->compaction_heap, lsm);
-	lsm->in_dump.pos = UINT32_MAX;
-	lsm->in_compaction.pos = UINT32_MAX;
 }
 
 static void
@@ -537,12 +535,12 @@ vy_scheduler_update_lsm(struct vy_scheduler *scheduler, struct vy_lsm *lsm)
 {
 	if (lsm->is_dropped) {
 		/* Dropped LSM trees are exempted from scheduling. */
-		assert(lsm->in_dump.pos == UINT32_MAX);
-		assert(lsm->in_compaction.pos == UINT32_MAX);
+		assert(heap_node_is_stray(&lsm->in_dump));
+		assert(heap_node_is_stray(&lsm->in_compaction));
 		return;
 	}
-	assert(lsm->in_dump.pos != UINT32_MAX);
-	assert(lsm->in_compaction.pos != UINT32_MAX);
+	assert(! heap_node_is_stray(&lsm->in_dump));
+	assert(! heap_node_is_stray(&lsm->in_compaction));
 	vy_dump_heap_update(&scheduler->dump_heap, lsm);
 	vy_compaction_heap_update(&scheduler->compaction_heap, lsm);
 }
@@ -1601,7 +1599,7 @@ vy_task_compaction_complete(struct vy_task *task)
 	/* The iterator has been cleaned up in worker. */
 	task->wi->iface->close(task->wi);
 
-	assert(range->heap_node.pos == UINT32_MAX);
+	assert(heap_node_is_stray(&range->heap_node));
 	vy_range_heap_insert(&lsm->range_heap, range);
 	vy_scheduler_update_lsm(scheduler, lsm);
 
@@ -1633,7 +1631,7 @@ vy_task_compaction_abort(struct vy_task *task)
 
 	vy_run_discard(task->new_run);
 
-	assert(range->heap_node.pos == UINT32_MAX);
+	assert(heap_node_is_stray(&range->heap_node));
 	vy_range_heap_insert(&lsm->range_heap, range);
 	vy_scheduler_update_lsm(scheduler, lsm);
 }
@@ -1722,7 +1720,6 @@ vy_task_compaction_new(struct vy_scheduler *scheduler, struct vy_worker *worker,
 	 * so that it doesn't get selected again.
 	 */
 	vy_range_heap_delete(&lsm->range_heap, range);
-	range->heap_node.pos = UINT32_MAX;
 	vy_scheduler_update_lsm(scheduler, lsm);
 
 	say_info("%s: started compacting range %s, runs %d/%d",
diff --git a/src/lib/salad/heap.h b/src/lib/salad/heap.h
index 6ba4a22ca..f267de3f1 100644
--- a/src/lib/salad/heap.h
+++ b/src/lib/salad/heap.h
@@ -126,7 +126,8 @@
 #define HEAP_STRUCTURES
 
 enum {
-	HEAP_INITIAL_CAPACITY = 8
+	HEAP_INITIAL_CAPACITY = 8,
+	HEAP_NODE_STRAY_POS = UINT32_MAX,
 };
 
 typedef uint32_t heap_off_t;
@@ -149,6 +150,20 @@ struct heap_node {
 	heap_off_t pos;
 };
 
+/** Initialize a heap node with default values. */
+static inline void
+heap_node_create(struct heap_node *node)
+{
+	node->pos = HEAP_NODE_STRAY_POS;
+}
+
+/** Check if a heap node does not belong to any heap. */
+static inline bool
+heap_node_is_stray(const struct heap_node *node)
+{
+	return node->pos == HEAP_NODE_STRAY_POS;
+}
+
 /**
  * Heap iterator structure.
  */
@@ -429,9 +444,11 @@ HEAP(delete)(heap_t *heap, heap_value_t *value)
 	if (heap->size == 0)
 		return;
 
+	struct heap_node *node = value_to_node(value);
+	assert(! heap_node_is_stray(node));
 	heap->size--;
-
-	heap_off_t curr_pos = value_to_node(value)->pos;
+	heap_off_t curr_pos = node->pos;
+	heap_node_create(node);
 
 	if (curr_pos == heap->size)
 		return;
-- 
2.17.2 (Apple Git-113)

  parent reply	other threads:[~2019-02-22 11:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-22 11:38 [PATCH 0/3] Make heap API more friendly Vladislav Shpilevoy
2019-02-22 11:38 ` [PATCH 1/3] salad: make heap struct more friendly to use Vladislav Shpilevoy
2019-02-22 18:26   ` [tarantool-patches] " Konstantin Osipov
2019-02-22 18:38     ` [tarantool-patches] " Vladislav Shpilevoy
2019-02-22 11:38 ` Vladislav Shpilevoy [this message]
2019-02-25 12:46   ` [PATCH 2/3] salad: do not touch struct heap_node.pos in user's code Vladimir Davydov
2019-02-22 11:39 ` [PATCH 3/3] salad: fix heap reserve() behaviour Vladislav Shpilevoy
2019-02-22 18:31   ` [tarantool-patches] " Konstantin Osipov
2019-02-22 18:38     ` [tarantool-patches] " Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=36f1e53ccc18d49524061ddc442cf6cb80e3ce4e.1550835301.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 2/3] salad: do not touch struct heap_node.pos in user'\''s code' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox