From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 1/7] Make tuple comparison hints mandatory Date: Wed, 8 May 2019 20:22:33 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: To: tarantool-patches@freelists.org List-ID: There isn't much point in having separate versions of tuple comparators that don't take tuple comparison hints anymore, because all hot paths have been patched to use the hinted versions. Besides un-hinted versions don't make sense for multikey indexes, which use hints to store offsets in multikey arrays. Let's strip the _hinted suffix from all hinted comparators and zap un-hinted versions. In a few remaining places in the code that still use un-hinted versions, let's simply pass HINT_NONE. --- src/box/key_def.c | 5 +- src/box/key_def.h | 99 ++++---------------- src/box/lua/key_def.c | 5 +- src/box/memtx_hash.c | 7 +- src/box/memtx_space.c | 4 +- src/box/memtx_tree.c | 41 ++++----- src/box/space.c | 3 +- src/box/sql/analyze.c | 5 +- src/box/tuple_compare.cc | 231 ++++++++++------------------------------------- src/box/vinyl.c | 9 +- src/box/vy_range.c | 3 +- src/box/vy_stmt.h | 81 ++++------------- src/box/vy_upsert.c | 3 +- 13 files changed, 133 insertions(+), 363 deletions(-) diff --git a/src/box/key_def.c b/src/box/key_def.c index d9e8e7a1..fc547570 100644 --- a/src/box/key_def.c +++ b/src/box/key_def.c @@ -350,7 +350,7 @@ int box_tuple_compare(box_tuple_t *tuple_a, box_tuple_t *tuple_b, box_key_def_t *key_def) { - return tuple_compare(tuple_a, tuple_b, key_def); + return tuple_compare(tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def); } int @@ -358,7 +358,8 @@ box_tuple_compare_with_key(box_tuple_t *tuple_a, const char *key_b, box_key_def_t *key_def) { uint32_t part_count = mp_decode_array(&key_b); - return tuple_compare_with_key(tuple_a, key_b, part_count, key_def); + return tuple_compare_with_key(tuple_a, HINT_NONE, key_b, + part_count, HINT_NONE, key_def); } diff --git a/src/box/key_def.h b/src/box/key_def.h index 7b3d03c2..5f65f501 100644 --- a/src/box/key_def.h +++ b/src/box/key_def.h @@ -130,27 +130,18 @@ key_part_is_nullable(const struct key_part *part) } /** @copydoc tuple_compare_with_key() */ -typedef int (*tuple_compare_with_key_t)(struct tuple *tuple_a, +typedef int (*tuple_compare_with_key_t)(struct tuple *tuple, + hint_t tuple_hint, const char *key, uint32_t part_count, + hint_t key_hint, struct key_def *key_def); -/** @copydoc tuple_compare_with_key_hinted() */ -typedef int (*tuple_compare_with_key_hinted_t)(struct tuple *tuple, - hint_t tuple_hint, - const char *key, - uint32_t part_count, - hint_t key_hint, - struct key_def *key_def); /** @copydoc tuple_compare() */ typedef int (*tuple_compare_t)(struct tuple *tuple_a, + hint_t tuple_a_hint, struct tuple *tuple_b, + hint_t tuple_b_hint, struct key_def *key_def); -/** @copydoc tuple_compare_hinted() */ -typedef int (*tuple_compare_hinted_t)(struct tuple *tuple_a, - hint_t tuple_a_hint, - struct tuple *tuple_b, - hint_t tuple_b_hint, - struct key_def *key_def); /** @copydoc tuple_extract_key() */ typedef char *(*tuple_extract_key_t)(struct tuple *tuple, struct key_def *key_def, @@ -177,12 +168,8 @@ typedef hint_t (*key_hint_t)(const char *key, uint32_t part_count, struct key_def { /** @see tuple_compare() */ tuple_compare_t tuple_compare; - /** @see tuple_compare_hinted() */ - tuple_compare_hinted_t tuple_compare_hinted; /** @see tuple_compare_with_key() */ tuple_compare_with_key_t tuple_compare_with_key; - /** @see tuple_compare_with_key_hinted() */ - tuple_compare_with_key_hinted_t tuple_compare_with_key_hinted; /** @see tuple_extract_key() */ tuple_extract_key_t tuple_extract_key; /** @see tuple_extract_key_raw() */ @@ -601,21 +588,6 @@ tuple_extract_key_raw(const char *data, const char *data_end, } /** - * Compare keys using the key definition. - * @param key_a key parts with MessagePack array header - * @param part_count_a the number of parts in the key_a - * @param key_b key_parts with MessagePack array header - * @param part_count_b the number of parts in the key_b - * @param key_def key definition - * - * @retval 0 if key_a == key_b - * @retval <0 if key_a < key_b - * @retval >0 if key_a > key_b - */ -int -key_compare(const char *key_a, const char *key_b, struct key_def *key_def); - -/** * Compare keys using the key definition and comparison hints. * @param key_a key parts with MessagePack array header * @param key_a_hint comparison hint of @a key_a @@ -628,25 +600,9 @@ key_compare(const char *key_a, const char *key_b, struct key_def *key_def); * @retval >0 if key_a > key_b */ int -key_compare_hinted(const char *key_a, hint_t key_a_hint, - const char *key_b, hint_t key_b_hint, - struct key_def *key_def); - -/** - * Compare tuples using the key definition. - * @param tuple_a first tuple - * @param tuple_b second tuple - * @param key_def key definition - * @retval 0 if key_fields(tuple_a) == key_fields(tuple_b) - * @retval <0 if key_fields(tuple_a) < key_fields(tuple_b) - * @retval >0 if key_fields(tuple_a) > key_fields(tuple_b) - */ -static inline int -tuple_compare(struct tuple *tuple_a, struct tuple *tuple_b, - struct key_def *key_def) -{ - return key_def->tuple_compare(tuple_a, tuple_b, key_def); -} +key_compare(const char *key_a, hint_t key_a_hint, + const char *key_b, hint_t key_b_hint, + struct key_def *key_def); /** * Compare tuples using the key definition and comparison hints. @@ -660,30 +616,12 @@ tuple_compare(struct tuple *tuple_a, struct tuple *tuple_b, * @retval >0 if key_fields(tuple_a) > key_fields(tuple_b) */ static inline int -tuple_compare_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, - struct tuple *tuple_b, hint_t tuple_b_hint, - struct key_def *key_def) +tuple_compare(struct tuple *tuple_a, hint_t tuple_a_hint, + struct tuple *tuple_b, hint_t tuple_b_hint, + struct key_def *key_def) { - return key_def->tuple_compare_hinted(tuple_a, tuple_a_hint, tuple_b, - tuple_b_hint, key_def); -} - -/** - * @brief Compare tuple with key using the key definition. - * @param tuple tuple - * @param key key parts without MessagePack array header - * @param part_count the number of parts in @a key - * @param key_def key definition - * - * @retval 0 if key_fields(tuple) == parts(key) - * @retval <0 if key_fields(tuple) < parts(key) - * @retval >0 if key_fields(tuple) > parts(key) - */ -static inline int -tuple_compare_with_key(struct tuple *tuple, const char *key, - uint32_t part_count, struct key_def *key_def) -{ - return key_def->tuple_compare_with_key(tuple, key, part_count, key_def); + return key_def->tuple_compare(tuple_a, tuple_a_hint, + tuple_b, tuple_b_hint, key_def); } /** @@ -700,13 +638,12 @@ tuple_compare_with_key(struct tuple *tuple, const char *key, * @retval >0 if key_fields(tuple) > parts(key) */ static inline int -tuple_compare_with_key_hinted(struct tuple *tuple, hint_t tuple_hint, - const char *key, uint32_t part_count, - hint_t key_hint, struct key_def *key_def) +tuple_compare_with_key(struct tuple *tuple, hint_t tuple_hint, + const char *key, uint32_t part_count, + hint_t key_hint, struct key_def *key_def) { - return key_def->tuple_compare_with_key_hinted(tuple, tuple_hint, key, - part_count, key_hint, - key_def); + return key_def->tuple_compare_with_key(tuple, tuple_hint, key, + part_count, key_hint, key_def); } /** diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index b4bd64f5..72bb05c4 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -305,7 +305,7 @@ lbox_key_def_compare(struct lua_State *L) return luaT_error(L); } - int rc = tuple_compare(tuple_a, tuple_b, key_def); + int rc = tuple_compare(tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def); tuple_unref(tuple_a); tuple_unref(tuple_b); lua_pushinteger(L, rc); @@ -345,7 +345,8 @@ lbox_key_def_compare_with_key(struct lua_State *L) return luaT_error(L); } - int rc = tuple_compare_with_key(tuple, key, part_count, key_def); + int rc = tuple_compare_with_key(tuple, HINT_NONE, key, + part_count, HINT_NONE, key_def); region_truncate(region, region_svp); tuple_unref(tuple); lua_pushinteger(L, rc); diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c index ff478bdc..3174ae2c 100644 --- a/src/box/memtx_hash.c +++ b/src/box/memtx_hash.c @@ -44,15 +44,16 @@ static inline bool memtx_hash_equal(struct tuple *tuple_a, struct tuple *tuple_b, struct key_def *key_def) { - return tuple_compare(tuple_a, tuple_b, key_def) == 0; + return tuple_compare(tuple_a, HINT_NONE, + tuple_b, HINT_NONE, key_def) == 0; } static inline bool memtx_hash_equal_key(struct tuple *tuple, const char *key, struct key_def *key_def) { - return tuple_compare_with_key(tuple, key, key_def->part_count, - key_def) == 0; + return tuple_compare_with_key(tuple, HINT_NONE, key, key_def->part_count, + HINT_NONE, key_def) == 0; } #define LIGHT_NAME _index diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c index c3c9ac79..4d5e7991 100644 --- a/src/box/memtx_space.c +++ b/src/box/memtx_space.c @@ -513,8 +513,8 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn, struct index *pk = space->index[0]; if (!key_update_can_be_skipped(pk->def->key_def->column_mask, column_mask) && - tuple_compare(old_tuple, stmt->new_tuple, - pk->def->key_def) != 0) { + tuple_compare(old_tuple, HINT_NONE, stmt->new_tuple, + HINT_NONE, pk->def->key_def) != 0) { /* Primary key is changed: log error and do nothing. */ diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, pk->def->name, space_name(space)); diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c index a01aed48..268ce405 100644 --- a/src/box/memtx_tree.c +++ b/src/box/memtx_tree.c @@ -80,11 +80,10 @@ memtx_tree_data_identical(const struct memtx_tree_data *a, #define BPS_TREE_BLOCK_SIZE (512) #define BPS_TREE_EXTENT_SIZE MEMTX_EXTENT_SIZE #define BPS_TREE_COMPARE(a, b, arg)\ - tuple_compare_hinted((&a)->tuple, (&a)->hint, (&b)->tuple,\ - (&b)->hint, arg) + tuple_compare((&a)->tuple, (&a)->hint, (&b)->tuple, (&b)->hint, arg) #define BPS_TREE_COMPARE_KEY(a, b, arg)\ - tuple_compare_with_key_hinted((&a)->tuple, (&a)->hint, (b)->key,\ - (b)->part_count, (b)->hint, arg) + tuple_compare_with_key((&a)->tuple, (&a)->hint, (b)->key,\ + (b)->part_count, (b)->hint, arg) #define BPS_TREE_IDENTICAL(a, b) memtx_tree_data_identical(&a, &b) #define bps_tree_elem_t struct memtx_tree_data #define bps_tree_key_t struct memtx_tree_key_data * @@ -125,8 +124,8 @@ memtx_tree_qcompare(const void* a, const void *b, void *c) const struct memtx_tree_data *data_a = a; const struct memtx_tree_data *data_b = b; struct key_def *key_def = c; - return tuple_compare_hinted(data_a->tuple, data_a->hint, data_b->tuple, - data_b->hint, key_def); + return tuple_compare(data_a->tuple, data_a->hint, data_b->tuple, + data_b->hint, key_def); } /* {{{ MemtxTree Iterators ****************************************/ @@ -248,11 +247,11 @@ tree_iterator_next_equal(struct iterator *iterator, struct tuple **ret) memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator); /* Use user key def to save a few loops. */ if (res == NULL || - tuple_compare_with_key_hinted(res->tuple, res->hint, - it->key_data.key, - it->key_data.part_count, - it->key_data.hint, - it->index_def->key_def) != 0) { + tuple_compare_with_key(res->tuple, res->hint, + it->key_data.key, + it->key_data.part_count, + it->key_data.hint, + it->index_def->key_def) != 0) { iterator->next = tree_iterator_dummie; it->current.tuple = NULL; *ret = NULL; @@ -281,11 +280,11 @@ tree_iterator_prev_equal(struct iterator *iterator, struct tuple **ret) memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator); /* Use user key def to save a few loops. */ if (res == NULL || - tuple_compare_with_key_hinted(res->tuple, res->hint, - it->key_data.key, - it->key_data.part_count, - it->key_data.hint, - it->index_def->key_def) != 0) { + tuple_compare_with_key(res->tuple, res->hint, + it->key_data.key, + it->key_data.part_count, + it->key_data.hint, + it->index_def->key_def) != 0) { iterator->next = tree_iterator_dummie; it->current.tuple = NULL; *ret = NULL; @@ -861,11 +860,11 @@ memtx_tree_index_build_array_deduplicate(struct memtx_tree_index *index) while (r_idx < index->build_array_size) { if (index->build_array[w_idx].tuple != index->build_array[r_idx].tuple || - tuple_compare_hinted(index->build_array[w_idx].tuple, - index->build_array[w_idx].hint, - index->build_array[r_idx].tuple, - index->build_array[r_idx].hint, - cmp_def) != 0) { + tuple_compare(index->build_array[w_idx].tuple, + index->build_array[w_idx].hint, + index->build_array[r_idx].tuple, + index->build_array[r_idx].hint, + cmp_def) != 0) { /* Do not override the element itself. */ if (++w_idx == r_idx) continue; diff --git a/src/box/space.c b/src/box/space.c index 54ba97fb..edfc5d60 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -455,7 +455,8 @@ space_before_replace(struct space *space, struct txn *txn, */ if (pk != NULL && request_changed && old_tuple != NULL && new_tuple != NULL && - tuple_compare(old_tuple, new_tuple, pk->def->key_def) != 0) { + tuple_compare(old_tuple, HINT_NONE, new_tuple, HINT_NONE, + pk->def->key_def) != 0) { diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, pk->def->name, space->def->name); rc = -1; diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index 0c02050a..d2d29c5f 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -1346,8 +1346,9 @@ static int sample_compare(const void *a, const void *b, void *arg) { struct key_def *def = (struct key_def *)arg; - return key_compare(((struct index_sample *) a)->sample_key, - ((struct index_sample *) b)->sample_key, def); + return key_compare(((struct index_sample *) a)->sample_key, HINT_NONE, + ((struct index_sample *) b)->sample_key, HINT_NONE, + def); } /** diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc index af9aa7b6..9a3780c6 100644 --- a/src/box/tuple_compare.cc +++ b/src/box/tuple_compare.cc @@ -375,29 +375,6 @@ mp_compare_scalar_coll(const char *field_a, const char *field_b, return mp_compare_scalar_with_type(field_a, type_a, field_b, type_b); } -static inline int -tuple_compare_multikey(struct tuple *tuple_a, struct tuple *tuple_b, - struct key_def *key_def) -{ - (void)tuple_a; - (void)tuple_b; - (void)key_def; - unreachable(); - return 0; -} - -static inline int -tuple_compare_with_key_multikey(struct tuple *tuple, const char *key, - uint32_t part_count, struct key_def *key_def) -{ - (void) tuple; - (void) key; - (void) part_count; - (void) key_def; - unreachable(); - return 0; -} - /** * @brief Compare two fields parts using a type definition * @param field_a field @@ -471,9 +448,9 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type, template static inline int -tuple_compare_slowpath_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, - struct tuple *tuple_b, hint_t tuple_b_hint, - struct key_def *key_def) +tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint, + struct tuple *tuple_b, hint_t tuple_b_hint, + struct key_def *key_def) { assert(has_json_paths == key_def->has_json_paths); assert(!has_optional_parts || is_nullable); @@ -621,22 +598,12 @@ tuple_compare_slowpath_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, return 0; } -template -static inline int -tuple_compare_slowpath(struct tuple *tuple_a, struct tuple *tuple_b, - struct key_def *key_def) -{ - return tuple_compare_slowpath_hinted - - (tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def); -} - template static inline int -tuple_compare_with_key_slowpath_hinted(struct tuple *tuple, - hint_t tuple_hint, const char *key, uint32_t part_count, - hint_t key_hint, struct key_def *key_def) +tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint, + const char *key, uint32_t part_count, + hint_t key_hint, struct key_def *key_def) { assert(has_json_paths == key_def->has_json_paths); assert(!has_optional_parts || is_nullable); @@ -731,16 +698,6 @@ tuple_compare_with_key_slowpath_hinted(struct tuple *tuple, return 0; } -template -static inline int -tuple_compare_with_key_slowpath(struct tuple *tuple, const char *key, - uint32_t part_count, struct key_def *key_def) -{ - return tuple_compare_with_key_slowpath_hinted - - (tuple, HINT_NONE, key, part_count, HINT_NONE, key_def); -} - template static inline int key_compare_parts(const char *key_a, const char *key_b, uint32_t part_count, @@ -799,9 +756,9 @@ key_compare_parts(const char *key_a, const char *key_b, uint32_t part_count, template static inline int -tuple_compare_with_key_sequential_hinted(struct tuple *tuple, - hint_t tuple_hint, const char *key, uint32_t part_count, - hint_t key_hint, struct key_def *key_def) +tuple_compare_with_key_sequential(struct tuple *tuple, hint_t tuple_hint, + const char *key, uint32_t part_count, + hint_t key_hint, struct key_def *key_def) { assert(!has_optional_parts || is_nullable); assert(key_def_is_sequential(key_def)); @@ -842,19 +799,13 @@ tuple_compare_with_key_sequential_hinted(struct tuple *tuple, return 0; } -template -static inline int -tuple_compare_with_key_sequential(struct tuple *tuple, const char *key, - uint32_t part_count, struct key_def *key_def) -{ - return tuple_compare_with_key_sequential_hinted - - (tuple, HINT_NONE, key, part_count, HINT_NONE, key_def); -} - int -key_compare(const char *key_a, const char *key_b, struct key_def *key_def) +key_compare(const char *key_a, hint_t key_a_hint, + const char *key_b, hint_t key_b_hint, struct key_def *key_def) { + int rc = hint_cmp(key_a_hint, key_b_hint); + if (rc != 0) + return rc; uint32_t part_count_a = mp_decode_array(&key_a); uint32_t part_count_b = mp_decode_array(&key_b); assert(part_count_a <= key_def->part_count); @@ -870,22 +821,11 @@ key_compare(const char *key_a, const char *key_b, struct key_def *key_def) } } -int -key_compare_hinted(const char *key_a, hint_t key_a_hint, - const char *key_b, hint_t key_b_hint, - struct key_def *key_def) -{ - int rc = hint_cmp(key_a_hint, key_b_hint); - if (rc != 0) - return rc; - return key_compare(key_a, key_b, key_def); -} - template static int -tuple_compare_sequential_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, - struct tuple *tuple_b, hint_t tuple_b_hint, - struct key_def *key_def) +tuple_compare_sequential(struct tuple *tuple_a, hint_t tuple_a_hint, + struct tuple *tuple_b, hint_t tuple_b_hint, + struct key_def *key_def) { assert(!has_optional_parts || is_nullable); assert(has_optional_parts == key_def->has_optional_parts); @@ -953,16 +893,6 @@ tuple_compare_sequential_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, return 0; } -template -static int -tuple_compare_sequential(struct tuple *tuple_a, struct tuple *tuple_b, - struct key_def *key_def) -{ - return tuple_compare_sequential_hinted - - (tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def); -} - template static inline int field_compare(const char **field_a, const char **field_b); @@ -1078,9 +1008,13 @@ struct FieldCompare template struct TupleCompare { - static int compare(struct tuple *tuple_a, struct tuple *tuple_b, + static int compare(struct tuple *tuple_a, hint_t tuple_a_hint, + struct tuple *tuple_b, hint_t tuple_b_hint, struct key_def *) { + int rc = hint_cmp(tuple_a_hint, tuple_b_hint); + if (rc != 0) + return rc; struct tuple_format *format_a = tuple_format(tuple_a); struct tuple_format *format_b = tuple_format(tuple_b); const char *field_a, *field_b; @@ -1092,23 +1026,17 @@ struct TupleCompare compare(tuple_a, tuple_b, format_a, format_b, field_a, field_b); } - - static int compare_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, - struct tuple *tuple_b, hint_t tuple_b_hint, - struct key_def *key_def) - { - int rc = hint_cmp(tuple_a_hint, tuple_b_hint); - if (rc != 0) - return rc; - return compare(tuple_a, tuple_b, key_def); - } }; template struct TupleCompare<0, TYPE, MORE_TYPES...> { - static int compare(struct tuple *tuple_a, struct tuple *tuple_b, + static int compare(struct tuple *tuple_a, hint_t tuple_a_hint, + struct tuple *tuple_b, hint_t tuple_b_hint, struct key_def *) { + int rc = hint_cmp(tuple_a_hint, tuple_b_hint); + if (rc != 0) + return rc; struct tuple_format *format_a = tuple_format(tuple_a); struct tuple_format *format_b = tuple_format(tuple_b); const char *field_a = tuple_data(tuple_a); @@ -1118,28 +1046,15 @@ struct TupleCompare<0, TYPE, MORE_TYPES...> { return FieldCompare<0, TYPE, MORE_TYPES...>::compare(tuple_a, tuple_b, format_a, format_b, field_a, field_b); } - - static int compare_hinted(struct tuple *tuple_a, hint_t tuple_a_hint, - struct tuple *tuple_b, hint_t tuple_b_hint, - struct key_def *key_def) - { - int rc = hint_cmp(tuple_a_hint, tuple_b_hint); - if (rc != 0) - return rc; - return compare(tuple_a, tuple_b, key_def); - } }; } /* end of anonymous namespace */ struct comparator_signature { tuple_compare_t f; - tuple_compare_hinted_t f_hinted; uint32_t p[64]; }; #define COMPARATOR(...) \ - { TupleCompare<__VA_ARGS__>::compare, \ - TupleCompare<__VA_ARGS__>::compare_hinted, \ - { __VA_ARGS__, UINT32_MAX } }, + { TupleCompare<__VA_ARGS__>::compare, { __VA_ARGS__, UINT32_MAX } }, /** * field1 no, field1 type, field2 no, field2 type, ... @@ -1277,12 +1192,16 @@ template struct TupleCompareWithKey { static int - compare(struct tuple *tuple, const char *key, uint32_t part_count, - struct key_def *key_def) + compare(struct tuple *tuple, hint_t tuple_hint, + const char *key, uint32_t part_count, + hint_t key_hint, struct key_def *key_def) { /* Part count can be 0 in wildcard searches. */ if (part_count == 0) return 0; + int rc = hint_cmp(tuple_hint, key_hint); + if (rc != 0) + return rc; struct tuple_format *format = tuple_format(tuple); const char *field = tuple_field_raw(format, tuple_data(tuple), tuple_field_map(tuple), @@ -1291,29 +1210,21 @@ struct TupleCompareWithKey compare(tuple, key, part_count, key_def, format, field); } - - static int - compare_hinted(struct tuple *tuple, hint_t tuple_hint, - const char *key, uint32_t part_count, hint_t key_hint, - struct key_def *key_def) - { - int rc = hint_cmp(tuple_hint, key_hint); - if (rc != 0) - return rc; - return compare(tuple, key, part_count, key_def); - } }; template struct TupleCompareWithKey<0, 0, TYPE, MORE_TYPES...> { - static int compare(struct tuple *tuple, + static int compare(struct tuple *tuple, hint_t tuple_hint, const char *key, uint32_t part_count, - struct key_def *key_def) + hint_t key_hint, struct key_def *key_def) { /* Part count can be 0 in wildcard searches. */ if (part_count == 0) return 0; + int rc = hint_cmp(tuple_hint, key_hint); + if (rc != 0) + return rc; struct tuple_format *format = tuple_format(tuple); const char *field = tuple_data(tuple); mp_decode_array(&field); @@ -1321,17 +1232,6 @@ struct TupleCompareWithKey<0, 0, TYPE, MORE_TYPES...> compare(tuple, key, part_count, key_def, format, field); } - - static int - compare_hinted(struct tuple *tuple, hint_t tuple_hint, - const char *key, uint32_t part_count, hint_t key_hint, - struct key_def *key_def) - { - int rc = hint_cmp(tuple_hint, key_hint); - if (rc != 0) - return rc; - return compare(tuple, key, part_count, key_def); - } }; } /* end of anonymous namespace */ @@ -1339,14 +1239,11 @@ struct TupleCompareWithKey<0, 0, TYPE, MORE_TYPES...> struct comparator_with_key_signature { tuple_compare_with_key_t f; - tuple_compare_with_key_hinted_t f_hinted; uint32_t p[64]; }; #define KEY_COMPARATOR(...) \ - { TupleCompareWithKey<0, __VA_ARGS__>::compare, \ - TupleCompareWithKey<0, __VA_ARGS__>::compare_hinted, \ - { __VA_ARGS__ } }, + { TupleCompareWithKey<0, __VA_ARGS__>::compare, { __VA_ARGS__ } }, static const comparator_with_key_signature cmp_wk_arr[] = { KEY_COMPARATOR(0, FIELD_TYPE_UNSIGNED, 1, FIELD_TYPE_UNSIGNED, 2, FIELD_TYPE_UNSIGNED) @@ -1680,9 +1577,8 @@ key_hint_multikey(const char *key, uint32_t part_count, struct key_def *key_def) /* * Multikey hint for tuple is an index of the key in * array, it always must be defined. While - * tuple_hint_multikey, tuple_compare_multikey and - * tuple_compare_with_key_multikey assume that it must - * be initialized manually(so they mustn't be called), + * tuple_hint_multikey assumes that it must be + * initialized manually (so it mustn't be called), * the virtual method for a key makes sense. Overriding * this method such way, we extend existend code to * do nothing on key hint calculation an it is valid @@ -1764,9 +1660,7 @@ key_def_set_compare_func_fast(struct key_def *def) assert(!key_def_has_collation(def)); tuple_compare_t cmp = NULL; - tuple_compare_hinted_t cmp_hinted = NULL; tuple_compare_with_key_t cmp_wk = NULL; - tuple_compare_with_key_hinted_t cmp_wk_hinted = NULL; bool is_sequential = key_def_is_sequential(def); /* @@ -1781,7 +1675,6 @@ key_def_set_compare_func_fast(struct key_def *def) break; if (i == def->part_count && cmp_arr[k].p[i * 2] == UINT32_MAX) { cmp = cmp_arr[k].f; - cmp_hinted = cmp_arr[k].f_hinted; break; } } @@ -1794,32 +1687,23 @@ key_def_set_compare_func_fast(struct key_def *def) } if (i == def->part_count) { cmp_wk = cmp_wk_arr[k].f; - cmp_wk_hinted = cmp_wk_arr[k].f_hinted; break; } } if (cmp == NULL) { cmp = is_sequential ? tuple_compare_sequential : - tuple_compare_slowpath; - cmp_hinted = is_sequential ? - tuple_compare_sequential_hinted : - tuple_compare_slowpath_hinted; + tuple_compare_slowpath; } if (cmp_wk == NULL) { cmp_wk = is_sequential ? tuple_compare_with_key_sequential : - tuple_compare_with_key_slowpath; - cmp_wk_hinted = is_sequential ? - tuple_compare_with_key_sequential_hinted : - tuple_compare_with_key_slowpath_hinted; + tuple_compare_with_key_slowpath; } def->tuple_compare = cmp; - def->tuple_compare_hinted = cmp_hinted; def->tuple_compare_with_key = cmp_wk; - def->tuple_compare_with_key_hinted = cmp_wk_hinted; } template @@ -1830,24 +1714,13 @@ key_def_set_compare_func_plain(struct key_def *def) if (key_def_is_sequential(def)) { def->tuple_compare = tuple_compare_sequential ; - def->tuple_compare_hinted = tuple_compare_sequential_hinted - ; def->tuple_compare_with_key = tuple_compare_with_key_sequential ; - def->tuple_compare_with_key_hinted = - tuple_compare_with_key_sequential_hinted - ; } else { def->tuple_compare = tuple_compare_slowpath - ; - def->tuple_compare_hinted = tuple_compare_slowpath_hinted ; def->tuple_compare_with_key = tuple_compare_with_key_slowpath - ; - def->tuple_compare_with_key_hinted = - tuple_compare_with_key_slowpath_hinted - ; + ; } } @@ -1857,22 +1730,14 @@ key_def_set_compare_func_json(struct key_def *def) { assert(def->has_json_paths); if (key_def_is_multikey(def)) { - def->tuple_compare = tuple_compare_multikey; - def->tuple_compare_hinted = tuple_compare_slowpath_hinted + def->tuple_compare = tuple_compare_slowpath ; - def->tuple_compare_with_key = tuple_compare_with_key_multikey; - def->tuple_compare_with_key_hinted = - tuple_compare_with_key_slowpath_hinted + def->tuple_compare_with_key = tuple_compare_with_key_slowpath ; } else { def->tuple_compare = tuple_compare_slowpath - ; - def->tuple_compare_hinted = tuple_compare_slowpath_hinted ; def->tuple_compare_with_key = tuple_compare_with_key_slowpath - ; - def->tuple_compare_with_key_hinted = - tuple_compare_with_key_slowpath_hinted ; } } diff --git a/src/box/vinyl.c b/src/box/vinyl.c index f63383f8..6ffbecd9 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -1366,7 +1366,8 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx, * we must not use vy_entry_compare() here. */ if (result->stmt == NULL || - vy_stmt_compare(result->stmt, entry.stmt, lsm->cmp_def) != 0) { + vy_stmt_compare(result->stmt, HINT_NONE, entry.stmt, + HINT_NONE, lsm->cmp_def) != 0) { /* * If a tuple read from a secondary index doesn't * match the tuple corresponding to it in the @@ -1622,7 +1623,8 @@ vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv, * fail here. */ if (found != NULL && vy_stmt_type(stmt) == IPROTO_REPLACE && - vy_stmt_compare(stmt, found, lsm->pk->key_def) == 0) { + vy_stmt_compare(stmt, HINT_NONE, found, HINT_NONE, + lsm->pk->key_def) == 0) { tuple_unref(found); return 0; } @@ -1827,7 +1829,8 @@ vy_check_update(struct space *space, const struct vy_lsm *pk, uint64_t column_mask) { if (!key_update_can_be_skipped(pk->key_def->column_mask, column_mask) && - vy_stmt_compare(old_tuple, new_tuple, pk->key_def) != 0) { + vy_stmt_compare(old_tuple, HINT_NONE, new_tuple, + HINT_NONE, pk->key_def) != 0) { diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY, index_name_by_id(space, pk->index_id), space_name(space)); diff --git a/src/box/vy_range.c b/src/box/vy_range.c index 3c36d345..395639dc 100644 --- a/src/box/vy_range.c +++ b/src/box/vy_range.c @@ -478,7 +478,8 @@ vy_range_needs_split(struct vy_range *range, int64_t range_size, slice->first_page_no); /* No point in splitting if a new range is going to be empty. */ - if (key_compare(first_page->min_key, mid_page->min_key, + if (key_compare(first_page->min_key, first_page->min_key_hint, + mid_page->min_key, mid_page->min_key_hint, range->cmp_def) == 0) return false; /* diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h index cc8a9c5d..49fa1602 100644 --- a/src/box/vy_stmt.h +++ b/src/box/vy_stmt.h @@ -380,91 +380,50 @@ vy_stmt_hint(struct tuple *stmt, struct key_def *key_def) /** * Compare two vinyl statements taking into account their - * formats (key or tuple). - */ -static inline int -vy_stmt_compare(struct tuple *a, struct tuple *b, struct key_def *key_def) -{ - bool a_is_tuple = !vy_stmt_is_key(a); - bool b_is_tuple = !vy_stmt_is_key(b); - if (a_is_tuple && b_is_tuple) { - return tuple_compare(a, b, key_def); - } else if (a_is_tuple && !b_is_tuple) { - const char *key = tuple_data(b); - uint32_t part_count = mp_decode_array(&key); - return tuple_compare_with_key(a, key, part_count, key_def); - } else if (!a_is_tuple && b_is_tuple) { - const char *key = tuple_data(a); - uint32_t part_count = mp_decode_array(&key); - return -tuple_compare_with_key(b, key, part_count, key_def); - } else { - assert(!a_is_tuple && !b_is_tuple); - return key_compare(tuple_data(a), tuple_data(b), key_def); - } -} - -/** - * Compare two vinyl statements taking into account their * formats (key or tuple) and using comparison hints. */ static inline int -vy_stmt_compare_hinted(struct tuple *a, hint_t a_hint, - struct tuple *b, hint_t b_hint, - struct key_def *key_def) +vy_stmt_compare(struct tuple *a, hint_t a_hint, + struct tuple *b, hint_t b_hint, + struct key_def *key_def) { bool a_is_tuple = !vy_stmt_is_key(a); bool b_is_tuple = !vy_stmt_is_key(b); if (a_is_tuple && b_is_tuple) { - return tuple_compare_hinted(a, a_hint, b, b_hint, key_def); + return tuple_compare(a, a_hint, b, b_hint, key_def); } else if (a_is_tuple && !b_is_tuple) { const char *key = tuple_data(b); uint32_t part_count = mp_decode_array(&key); - return tuple_compare_with_key_hinted(a, a_hint, key, part_count, - b_hint, key_def); + return tuple_compare_with_key(a, a_hint, key, part_count, + b_hint, key_def); } else if (!a_is_tuple && b_is_tuple) { const char *key = tuple_data(a); uint32_t part_count = mp_decode_array(&key); - return -tuple_compare_with_key_hinted(b, b_hint, key, part_count, - a_hint, key_def); + return -tuple_compare_with_key(b, b_hint, key, part_count, + a_hint, key_def); } else { assert(!a_is_tuple && !b_is_tuple); - return key_compare_hinted(tuple_data(a), a_hint, - tuple_data(b), b_hint, key_def); + return key_compare(tuple_data(a), a_hint, + tuple_data(b), b_hint, key_def); } } /** * Compare a vinyl statement (key or tuple) with a raw key - * (msgpack array). - */ -static inline int -vy_stmt_compare_with_raw_key(struct tuple *stmt, const char *key, - struct key_def *key_def) -{ - if (!vy_stmt_is_key(stmt)) { - uint32_t part_count = mp_decode_array(&key); - return tuple_compare_with_key(stmt, key, part_count, key_def); - } - return key_compare(tuple_data(stmt), key, key_def); -} - -/** - * Compare a vinyl statement (key or tuple) with a raw key * (msgpack array) using comparison hints. */ static inline int -vy_stmt_compare_with_raw_key_hinted(struct tuple *stmt, hint_t stmt_hint, - const char *key, hint_t key_hint, - struct key_def *key_def) +vy_stmt_compare_with_raw_key(struct tuple *stmt, hint_t stmt_hint, + const char *key, hint_t key_hint, + struct key_def *key_def) { if (!vy_stmt_is_key(stmt)) { uint32_t part_count = mp_decode_array(&key); - return tuple_compare_with_key_hinted(stmt, stmt_hint, - key, part_count, key_hint, - key_def); + return tuple_compare_with_key(stmt, stmt_hint, key, + part_count, key_hint, + key_def); } - return key_compare_hinted(tuple_data(stmt), stmt_hint, - key, key_hint, key_def); + return key_compare(tuple_data(stmt), stmt_hint, key, key_hint, key_def); } /** @@ -761,7 +720,7 @@ vy_entry_key_from_msgpack(struct tuple_format *format, struct key_def *key_def, static inline int vy_entry_compare(struct vy_entry a, struct vy_entry b, struct key_def *key_def) { - return vy_stmt_compare_hinted(a.stmt, a.hint, b.stmt, b.hint, key_def); + return vy_stmt_compare(a.stmt, a.hint, b.stmt, b.hint, key_def); } /** @@ -773,8 +732,8 @@ vy_entry_compare_with_raw_key(struct vy_entry entry, const char *key, hint_t key_hint, struct key_def *key_def) { - return vy_stmt_compare_with_raw_key_hinted(entry.stmt, entry.hint, - key, key_hint, key_def); + return vy_stmt_compare_with_raw_key(entry.stmt, entry.hint, + key, key_hint, key_def); } #if defined(__cplusplus) diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c index 2c693706..e5e53be7 100644 --- a/src/box/vy_upsert.c +++ b/src/box/vy_upsert.c @@ -206,7 +206,8 @@ check_key: * Check that key hasn't been changed after applying operations. */ if (!key_update_can_be_skipped(cmp_def->column_mask, column_mask) && - vy_stmt_compare(old_stmt, result_stmt, cmp_def) != 0) { + vy_stmt_compare(old_stmt, HINT_NONE, result_stmt, + HINT_NONE, cmp_def) != 0) { /* * Key has been changed: ignore this UPSERT and * @retval the old stmt. -- 2.11.0