From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 25 Feb 2019 19:57:58 +0300 From: Vladimir Davydov Subject: Re: [PATCH v3 5/7] memtx: rework memtx_tree to store arbitrary nodes Message-ID: <20190225165758.z4tkrqts3lgio7be@esperanza> References: <9bc3c05ed8934b2d2ef00b6d570eafce4708c787.1550849496.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9bc3c05ed8934b2d2ef00b6d570eafce4708c787.1550849496.git.kshcherbatov@tarantool.org> To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org List-ID: On Fri, Feb 22, 2019 at 06:42:30PM +0300, Kirill Shcherbatov wrote: > diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt > index 5521e489e..dc8cc2cd5 100644 > --- a/src/box/CMakeLists.txt > +++ b/src/box/CMakeLists.txt > @@ -64,7 +64,7 @@ add_library(box STATIC > index_def.c > iterator_type.c > memtx_hash.c > - memtx_tree.c > + memtx_tree_decl.c > memtx_rtree.c > memtx_bitset.c > engine.c > diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c > index 07d20473f..c48f96bf6 100644 > --- a/src/box/memtx_tree.c > +++ b/src/box/memtx_tree.c > @@ -152,25 +303,27 @@ tree_iterator_dummie(struct iterator *iterator, struct tuple **ret) > static int > tree_iterator_next(struct iterator *iterator, struct tuple **ret) > { > - struct tuple **res; > - struct tree_iterator *it = tree_iterator(iterator); > - assert(it->current_tuple != NULL); > - struct tuple **check = memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator); > - if (check == NULL || *check != it->current_tuple) > + struct tree_iterator *it = tree_iterator_cast(iterator); > + assert(MEMTX_TREE_ELEM_GET(&it->current) != NULL); > + memtx_tree_elem_t *check = > + memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator); > + if (check == NULL || !MEMTX_TREE_IDENTICAL(check, &it->current)) { > it->tree_iterator = > - memtx_tree_upper_bound_elem(it->tree, it->current_tuple, > - NULL); > - else > + memtx_tree_upper_bound_elem(it->tree, it->current, NULL); > + } else { > memtx_tree_iterator_next(it->tree, &it->tree_iterator); > - tuple_unref(it->current_tuple); > - it->current_tuple = NULL; > - res = memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator); > + } > + tuple_unref(MEMTX_TREE_ELEM_GET(&it->current)); > + memtx_tree_elem_t *res = > + memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator); > if (res == NULL) { > iterator->next = tree_iterator_dummie; > + memset(&it->current, 0, sizeof(it->current)); This looks like encapsulation violation. Please either use MEMTX_TREE_ELEM_SET for this or introduce MEMTX_TREE_ELEM_CLEAR helper. > @@ -490,26 +653,50 @@ memtx_tree_index_get(struct index *base, const char *key, > assert(base->def->opts.is_unique && > part_count == base->def->key_def->part_count); > struct memtx_tree_index *index = (struct memtx_tree_index *)base; > - struct memtx_tree_key_data key_data; > - key_data.key = key; > - key_data.part_count = part_count; > - struct tuple **res = memtx_tree_find(&index->tree, &key_data); > - *result = res != NULL ? *res : NULL; > + memtx_tree_key_t key_data; > + struct key_def *cmp_def = memtx_tree_index_cmp_def(index); > + MEMTX_TREE_KEY_SET(&key_data, key, part_count, cmp_def); > + memtx_tree_elem_t *res = memtx_tree_find(&index->tree, &key_data); > + *result = res != NULL ? MEMTX_TREE_ELEM_GET(res) : NULL; > return 0; > } > > +static int > +memtx_tree_index_insert_tuple(struct index *base, struct tuple *tuple, > + struct tuple **replaced) > +{ > + struct memtx_tree_index *index = (struct memtx_tree_index *)base; > + memtx_tree_elem_t data; > + MEMTX_TREE_ELEM_SET(&data, tuple, index->tree.arg); How is this going to work for multikey indexes? > + memtx_tree_elem_t data_replaced; > + memset(&data_replaced, 0, sizeof(data_replaced)); > + int rc = memtx_tree_insert(&index->tree, data, &data_replaced); > + if (replaced != NULL) > + *replaced = MEMTX_TREE_ELEM_GET(&data_replaced); > + return rc; > +} > + > +static void > +memtx_tree_index_delete_tuple(struct index *base, struct tuple *tuple) > +{ > + struct memtx_tree_index *index = (struct memtx_tree_index *)base; > + memtx_tree_elem_t data; > + MEMTX_TREE_ELEM_SET(&data, tuple, index->tree.arg); > + memtx_tree_delete(&index->tree, data); > +} > + > static int > memtx_tree_index_replace(struct index *base, struct tuple *old_tuple, > struct tuple *new_tuple, enum dup_replace_mode mode, > struct tuple **result) > { > - struct memtx_tree_index *index = (struct memtx_tree_index *)base; > if (new_tuple) { > struct tuple *dup_tuple = NULL; > > /* Try to optimistically replace the new_tuple. */ > - int tree_res = memtx_tree_insert(&index->tree, > - new_tuple, &dup_tuple); > + int tree_res = > + memtx_tree_index_insert_tuple(base, new_tuple, > + &dup_tuple); > if (tree_res) { > diag_set(OutOfMemory, MEMTX_EXTENT_SIZE, > "memtx_tree_index", "replace"); > @@ -519,9 +706,9 @@ memtx_tree_index_replace(struct index *base, struct tuple *old_tuple, > uint32_t errcode = replace_check_dup(old_tuple, > dup_tuple, mode); > if (errcode) { > - memtx_tree_delete(&index->tree, new_tuple); > + memtx_tree_index_delete_tuple(base, new_tuple); > if (dup_tuple) > - memtx_tree_insert(&index->tree, dup_tuple, 0); > + memtx_tree_index_insert_tuple(base, dup_tuple, 0); > struct space *sp = space_cache_find(base->def->space_id); > if (sp != NULL) > diag_set(ClientError, errcode, base->def->name, > @@ -533,9 +720,8 @@ memtx_tree_index_replace(struct index *base, struct tuple *old_tuple, > return 0; > } > } > - if (old_tuple) { > - memtx_tree_delete(&index->tree, old_tuple); > - } > + if (old_tuple) > + memtx_tree_index_delete_tuple(base, old_tuple); > *result = old_tuple; > return 0; > }