From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 BE07545C304 for ; Thu, 10 Dec 2020 20:25:05 +0300 (MSK) Date: Thu, 10 Dec 2020 17:25:04 +0000 From: Nikita Pettik Message-ID: <20201210172504.GD1319@tarantool.org> References: <2cf30fdbec386295dd853a8e94ade5582ddae8b6.1607075291.git.sergepetrenko@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2cf30fdbec386295dd853a8e94ade5582ddae8b6.1607075291.git.sergepetrenko@tarantool.org> 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: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org 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.