From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Kirill Shcherbatov <kshcherbatov@tarantool.org> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH v5 1/4] memtx: rework memtx_tree to store arbitrary nodes Date: Mon, 11 Mar 2019 13:34:01 +0300 [thread overview] Message-ID: <20190311103401.fs4cuvthwpiclx7d@esperanza> (raw) In-Reply-To: <7a26c07df4a10c5e67fe82fcd235b2521547c657.1551951540.git.kshcherbatov@tarantool.org> On Thu, Mar 07, 2019 at 12:44:05PM +0300, Kirill Shcherbatov wrote: > Reworked memtx_tree class to use arbitrary containers as a tree > nodes. This makes possible to implement tuple hints in subsequent > patches. The comment and the subject line are confusing: what arbitrary containers are you talking about? Looks like it's left from the previous version of the patch that used templates. Please fix. > > Needed for #3961 > --- > src/box/memtx_tree.c | 289 ++++++++++++++++++++++++++----------------- > 1 file changed, 178 insertions(+), 111 deletions(-) > > diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c > index 250df7f2d..688f09597 100644 > --- a/src/box/memtx_tree.c > +++ b/src/box/memtx_tree.c > @@ -50,30 +50,70 @@ struct memtx_tree_key_data { > }; > > /** > - * BPS tree element vs key comparator. > - * Defined in header in order to allow compiler to inline it. > - * @param tuple - tuple to compare. > - * @param key_data - key to compare with. > - * @param def - key definition. > - * @retval 0 if tuple == key in terms of def. > - * @retval <0 if tuple < key in terms of def. > - * @retval >0 if tuple > key in terms of def. > + * Struct that is used as a elem in BPS tree definition. > */ > -static int > -memtx_tree_compare_key(const struct tuple *tuple, > - const struct memtx_tree_key_data *key_data, > - struct key_def *def) > +struct memtx_tree_data { > + /* Tuple that this node is represents. */ > + struct tuple *tuple; > +}; > + > +/** > + * Test whether BPS tree elements are identical i.e. represents s/represents/represent > + * the same tuple and metadata. the same tuple at the same position in the tree. > + * @param a - First BPS tree element to compare. > + * @param b - Second BPS tree element to compare. > + * @retval true - When elements a and b are identical. > + * @retval fa;se - Otherwise. s/fa;se/false > + */ > +static bool > +memtx_tree_data_identical(const struct memtx_tree_data *a, > + const struct memtx_tree_data *b) > { > - return tuple_compare_with_key(tuple, key_data->key, > - key_data->part_count, def); > + return a->tuple == b->tuple; > +} > + > +/** > + * BPS tree element clear. > + * @param data - BPS tree element to clear. > + */ > +static void > +memtx_tree_data_clear(struct memtx_tree_data *data) > +{ > + data->tuple = NULL; > +} Please remove this trivial helper function - even after multikey indexes are introduced, this function will remain the same. > + > +/** > + * Initialize BPS tree element. > + */ > +static void > +memtx_tree_data_set(struct memtx_tree_data *data, struct tuple *tuple, > + struct key_def *key_def) > +{ > + (void)key_def; > + data->tuple = tuple; > +} Please remove this trivial helper function as well, as it's only used in 'build_next' and 'replace' index callbacks, which will differ for hinted and multikey trees anyway. > + > +/** > + * Initialize BPS tree key element. > + */ > +static void > +memtx_tree_key_data_set(struct memtx_tree_key_data *key_data, const char *key, > + uint32_t part_count, struct key_def *key_def) This function is only used in 'get' and 'create_iterator' index callbacks. Rather than using it, you should set different callbacks for multikey and hinted trees. Please remove it, too. > +{ > + (void)key_def; > + key_data->key = key; > + key_data->part_count = part_count; > } > @@ -614,21 +678,23 @@ static int > memtx_tree_index_build_next(struct index *base, struct tuple *tuple) > { > struct memtx_tree_index *index = (struct memtx_tree_index *)base; > + struct key_def *key_def = index->tree.arg; > if (index->build_array == NULL) { > - index->build_array = (struct tuple **)malloc(MEMTX_EXTENT_SIZE); > + index->build_array = > + (struct memtx_tree_data *)malloc(MEMTX_EXTENT_SIZE); Nit: pointless type cast, please remove.
next prev parent reply other threads:[~2019-03-11 10:34 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-07 9:44 [PATCH v5 0/4] box: introduce hint option for memtx tree index Kirill Shcherbatov 2019-03-07 9:44 ` [PATCH v5 1/4] memtx: rework memtx_tree to store arbitrary nodes Kirill Shcherbatov 2019-03-11 10:34 ` Vladimir Davydov [this message] 2019-03-11 16:53 ` [tarantool-patches] " Kirill Shcherbatov 2019-03-12 10:45 ` Vladimir Davydov 2019-03-07 9:44 ` [PATCH v5 2/4] memtx: introduce tuple compare hint Kirill Shcherbatov 2019-03-07 10:42 ` [tarantool-patches] " Konstantin Osipov 2019-03-07 10:59 ` Vladimir Davydov 2019-03-11 10:39 ` Vladimir Davydov 2019-03-11 17:03 ` Vladimir Davydov 2019-03-12 13:00 ` Vladimir Davydov 2019-03-07 9:44 ` [PATCH v5 3/4] box: move offset_slot init to tuple_format_add_field Kirill Shcherbatov 2019-03-07 15:53 ` [tarantool-patches] " Kirill Shcherbatov 2019-03-07 9:44 ` [PATCH v5 4/4] box: introduce multikey indexes Kirill Shcherbatov 2019-03-07 15:55 ` [tarantool-patches] " Kirill Shcherbatov 2019-03-12 13:24 ` Vladimir Davydov 2019-03-07 10:45 ` [tarantool-patches] [PATCH v5 0/4] box: introduce hint option for memtx tree index Konstantin Osipov
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=20190311103401.fs4cuvthwpiclx7d@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH v5 1/4] memtx: rework memtx_tree to store arbitrary nodes' \ /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