From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [94.100.177.91]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id CDFA4469719 for ; Sat, 14 Nov 2020 20:28:36 +0300 (MSK) From: Serge Petrenko Date: Sat, 14 Nov 2020 20:28:23 +0300 Message-Id: <20201114172823.8217-3-sergepetrenko@tarantool.org> In-Reply-To: <20201114172823.8217-1-sergepetrenko@tarantool.org> References: <20201114172823.8217-1-sergepetrenko@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 2/2] box: use tuple_field_raw_by_part where possible List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org, korablev@tarantool.org Cc: tarantool-patches@dev.tarantool.org tuple_field_raw_by_part allows to use part's offset slot cache to bypass tuple field lookup in a JSON tree, which is involved even for plain tuples. Since both tuple_field_raw_by_part and tuple_field_raw are basically aliases to tuple_field_raw_by_path, prefer the former to the latter, since it takes advantage over key part's offset slot cache. Also remove has_json_paths template argument for functions where it was used to decide between tuple_field_raw and tuple_field_raw_by_part. This patch was tested by snapshot recovery part involving secondary index building for a 1.5G snapshot with one space and one secondary index over 4 integer and one string field. Comparison table is below: Version | time(seconds) | Change relative to 1.10 ---------------|----------------|------------------------ 1.10 | 2:24 | -/- 2.x(unpatched) | 3:03 | + 27% 2.x (patched) | 2:44 | + 14% Numbers below show cumulative time spent in tuple_compare_slowpath, for 1.10 / 2.x(unpatched) / 2.x(patched) for 15, 19 and 17 second profiles respectively: 13.9 / 17.8 / 15.7. tuple_field_raw_by_path wasn't measured, since it's inlined, but all its uses come from tuple_compare_slowpath. Closes #4774 --- src/box/tuple_compare.cc | 37 +++++++++--------------------------- src/box/tuple_extract_key.cc | 10 ++-------- src/box/tuple_hash.cc | 35 +++++++++------------------------- 3 files changed, 20 insertions(+), 62 deletions(-) diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc index 0946d77f8..cbb3a850d 100644 --- a/src/box/tuple_compare.cc +++ b/src/box/tuple_compare.cc @@ -589,18 +589,13 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint, field_b = tuple_field_raw_by_part(format_b, tuple_b_raw, field_map_b, part, (int)tuple_b_hint); - } else if (has_json_paths) { + } else { field_a = tuple_field_raw_by_part(format_a, tuple_a_raw, field_map_a, part, MULTIKEY_NONE); field_b = tuple_field_raw_by_part(format_b, tuple_b_raw, field_map_b, part, MULTIKEY_NONE); - } else { - field_a = tuple_field_raw(format_a, tuple_a_raw, - field_map_a, part->fieldno); - field_b = tuple_field_raw(format_b, tuple_b_raw, - field_map_b, part->fieldno); } assert(has_optional_parts || (field_a != NULL && field_b != NULL)); @@ -655,18 +650,13 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint, field_b = tuple_field_raw_by_part(format_b, tuple_b_raw, field_map_b, part, (int)tuple_b_hint); - } else if (has_json_paths) { + } else { field_a = tuple_field_raw_by_part(format_a, tuple_a_raw, field_map_a, part, MULTIKEY_NONE); field_b = tuple_field_raw_by_part(format_b, tuple_b_raw, field_map_b, part, MULTIKEY_NONE); - } else { - field_a = tuple_field_raw(format_a, tuple_a_raw, - field_map_a, part->fieldno); - field_b = tuple_field_raw(format_b, tuple_b_raw, - field_map_b, part->fieldno); } /* * Extended parts are primary, and they can not @@ -681,14 +671,12 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint, return 0; } -template +template static inline int 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); assert(is_nullable == key_def->is_nullable); assert(has_optional_parts == key_def->has_optional_parts); @@ -711,13 +699,10 @@ tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint, field = tuple_field_raw_by_part(format, tuple_raw, field_map, part, (int)tuple_hint); - } else if (has_json_paths) { + } else { field = tuple_field_raw_by_part(format, tuple_raw, field_map, part, MULTIKEY_NONE); - } else { - field = tuple_field_raw(format, tuple_raw, field_map, - part->fieldno); } if (! is_nullable) { return tuple_compare_field(field, key, part->type, @@ -746,13 +731,10 @@ tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint, field = tuple_field_raw_by_part(format, tuple_raw, field_map, part, (int)tuple_hint); - } else if (has_json_paths) { + } else { field = tuple_field_raw_by_part(format, tuple_raw, field_map, part, MULTIKEY_NONE); - } else { - field = tuple_field_raw(format, tuple_raw, field_map, - part->fieldno); } if (! is_nullable) { rc = tuple_compare_field(field, key, part->type, @@ -1998,8 +1980,7 @@ key_def_set_compare_func_fast(struct key_def *def) if (cmp_wk == NULL) { cmp_wk = is_sequential ? tuple_compare_with_key_sequential : - tuple_compare_with_key_slowpath; + tuple_compare_with_key_slowpath; } def->tuple_compare = cmp; @@ -2020,7 +2001,7 @@ key_def_set_compare_func_plain(struct key_def *def) def->tuple_compare = tuple_compare_slowpath ; def->tuple_compare_with_key = tuple_compare_with_key_slowpath - ; + ; } } @@ -2033,12 +2014,12 @@ key_def_set_compare_func_json(struct key_def *def) def->tuple_compare = tuple_compare_slowpath ; def->tuple_compare_with_key = tuple_compare_with_key_slowpath - ; + ; } else { def->tuple_compare = tuple_compare_slowpath ; def->tuple_compare_with_key = tuple_compare_with_key_slowpath - ; + ; } } diff --git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc index c1ad3929e..6f347ac16 100644 --- a/src/box/tuple_extract_key.cc +++ b/src/box/tuple_extract_key.cc @@ -132,10 +132,7 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def, /* Calculate the key size. */ for (uint32_t i = 0; i < part_count; ++i) { const char *field; - if (!has_json_paths) { - field = tuple_field_raw(format, data, field_map, - key_def->parts[i].fieldno); - } else if (!is_multikey) { + if (!is_multikey) { field = tuple_field_raw_by_part(format, data, field_map, &key_def->parts[i], MULTIKEY_NONE); @@ -184,10 +181,7 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def, char *key_buf = mp_encode_array(key, part_count); for (uint32_t i = 0; i < part_count; ++i) { const char *field; - if (!has_json_paths) { - field = tuple_field_raw(format, data, field_map, - key_def->parts[i].fieldno); - } else if (!is_multikey) { + if (!is_multikey) { field = tuple_field_raw_by_part(format, data, field_map, &key_def->parts[i], MULTIKEY_NONE); diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc index 39f89a659..43e37af87 100644 --- a/src/box/tuple_hash.cc +++ b/src/box/tuple_hash.cc @@ -217,7 +217,7 @@ static const hasher_signature hash_arr[] = { #undef HASHER -template +template uint32_t tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def); @@ -261,15 +261,9 @@ key_def_set_hash_func(struct key_def *key_def) { slowpath: if (key_def->has_optional_parts) { - if (key_def->has_json_paths) - key_def->tuple_hash = tuple_hash_slowpath; - else - key_def->tuple_hash = tuple_hash_slowpath; + key_def->tuple_hash = tuple_hash_slowpath; } else { - if (key_def->has_json_paths) - key_def->tuple_hash = tuple_hash_slowpath; - else - key_def->tuple_hash = tuple_hash_slowpath; + key_def->tuple_hash = tuple_hash_slowpath; } key_def->key_hash = key_hash_slowpath; } @@ -358,11 +352,10 @@ tuple_hash_key_part(uint32_t *ph1, uint32_t *pcarry, struct tuple *tuple, return tuple_hash_field(ph1, pcarry, &field, part->coll); } -template +template uint32_t tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def) { - assert(has_json_paths == key_def->has_json_paths); assert(has_optional_parts == key_def->has_optional_parts); assert(!key_def->is_multikey); assert(!key_def->for_func_index); @@ -374,13 +367,8 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def) const char *tuple_raw = tuple_data(tuple); const uint32_t *field_map = tuple_field_map(tuple); const char *field; - if (has_json_paths) { - field = tuple_field_raw_by_part(format, tuple_raw, field_map, - key_def->parts, MULTIKEY_NONE); - } else { - field = tuple_field_raw(format, tuple_raw, field_map, - prev_fieldno); - } + field = tuple_field_raw_by_part(format, tuple_raw, field_map, + key_def->parts, MULTIKEY_NONE); const char *end = (char *)tuple + tuple_size(tuple); if (has_optional_parts && field == NULL) { total_size += tuple_hash_null(&h, &carry); @@ -395,14 +383,9 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def) */ if (prev_fieldno + 1 != key_def->parts[part_id].fieldno) { struct key_part *part = &key_def->parts[part_id]; - if (has_json_paths) { - field = tuple_field_raw_by_part(format, tuple_raw, - field_map, part, - MULTIKEY_NONE); - } else { - field = tuple_field_raw(format, tuple_raw, field_map, - part->fieldno); - } + field = tuple_field_raw_by_part(format, tuple_raw, + field_map, part, + MULTIKEY_NONE); } if (has_optional_parts && (field == NULL || field >= end)) { total_size += tuple_hash_null(&h, &carry); -- 2.24.3 (Apple Git-128)