From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (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 AE72C45C304 for ; Fri, 11 Dec 2020 09:36:55 +0300 (MSK) References: <2cf30fdbec386295dd853a8e94ade5582ddae8b6.1607075291.git.sergepetrenko@tarantool.org> <20201210172504.GD1319@tarantool.org> From: Serge Petrenko Message-ID: <5fef50a2-8364-de97-6c47-206f883bd894@tarantool.org> Date: Fri, 11 Dec 2020 09:36:54 +0300 MIME-Version: 1.0 In-Reply-To: <20201210172504.GD1319@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH v3 2/2] box: refactor tuple_field_raw to omit path checks List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org 10.12.2020 20:25, Nikita Pettik пишет: > On 04 Dec 12:52, Serge Petrenko wrote: >> tuple_field_raw is an alias to tuple_field_raw_by_path with zero path. >> This involves multiple path != NULL checks which aren't needed for tuple >> field access by field number. The checks make this function rather slow >> compared to its 1.10 counterpart (see results below). >> >> In order to fix perf problems when JSON path indices aren't involved, >> factor out the part of tuple_field_raw_by_path which is responsible for >> direct field access by number and place it in tuple_field_raw. >> >> 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:10 | - 10% >> >> Numbers below show cumulative time spent in tuple_compare_slowpath, >> for 1.10 / 2.x(unpatched) / 2.x(patched) for 15, 19 and 14 second >> profiles respectively: 13.9 / 17.8 / 12.5. >> >> tuple_field_raw() isn't measured directly, since it's inlined, but all >> its uses come from tuple_compare_slowpath. >> >> As the results show, we manage to be even faster, than 1.10 used to be >> in this test. This must be due to tuple comparison hints, which are >> present only in 2.x. >> >> Closes #4774 > LGTM > >> --- >> src/box/tuple.h | 29 +++++++++++++++++++++++++++-- >> 1 file changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/src/box/tuple.h b/src/box/tuple.h >> index 755aee506..fd373fdbf 100644 >> --- a/src/box/tuple.h >> +++ b/src/box/tuple.h >> @@ -697,8 +697,33 @@ static inline const char * >> tuple_field_raw(struct tuple_format *format, const char *tuple, >> const uint32_t *field_map, uint32_t field_no) >> { >> - return tuple_field_raw_by_path(format, tuple, field_map, field_no, >> - NULL, 0, NULL, MULTIKEY_NONE); >> + if (likely(field_no < format->index_field_count)) { >> + int32_t offset_slot; >> + uint32_t offset = 0; >> + struct tuple_field *field; >> + if (field_no == 0) { >> + mp_decode_array(&tuple); >> + return tuple; >> + } >> + struct json_token *token = format->fields.root.children[field_no]; >> + field = json_tree_entry(token, struct tuple_field, token); >> + offset_slot = field->offset_slot; >> + if (offset_slot == TUPLE_OFFSET_SLOT_NIL) >> + goto parse; >> + offset = field_map_get_offset(field_map, offset_slot, MULTIKEY_NONE); > Nit: these lines a bit break 80 border. I'd fix this. Ok, here: diff --git a/src/box/tuple.h b/src/box/tuple.h index fd373fdbf..e4267a4ec 100644 --- a/src/box/tuple.h +++ b/src/box/tuple.h @@ -710,7 +710,8 @@ tuple_field_raw(struct tuple_format *format, const char *tuple,                 offset_slot = field->offset_slot;                 if (offset_slot == TUPLE_OFFSET_SLOT_NIL)                         goto parse; -               offset = field_map_get_offset(field_map, offset_slot, MULTIKEY_NONE); +               offset = field_map_get_offset(field_map, offset_slot, +                                             MULTIKEY_NONE);                 if (offset == 0)                         return NULL;                 tuple += offset; I wasn't sure how to fix the `struct json_token *token = format->fields.root.children[field_no];` line, so I left it as is. > -- Serge Petrenko