From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (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 5BAE945C304 for ; Wed, 2 Dec 2020 13:08:01 +0300 (MSK) References: <245a5084-7f35-ea9c-6259-a7d260b300a0@tarantool.org> From: Serge Petrenko Message-ID: Date: Wed, 2 Dec 2020 13:07:59 +0300 MIME-Version: 1.0 In-Reply-To: <245a5084-7f35-ea9c-6259-a7d260b300a0@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH v2 1/2] box: speed up tuple_field_map_create List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , korablev@tarantool.org Cc: tarantool-patches@dev.tarantool.org 02.12.2020 01:01, Vladislav Shpilevoy пишет: > Hi! Thanks for the patch! > > On 30.11.2020 12:14, Serge Petrenko via Tarantool-patches wrote: >> Since the introduction of JSON path indices tuple_init_field_map, which >> was quite a simple routine traversing a tuple and storing its field data >> offsets in the field map, was renamed to tuple_field_map_create and >> optimised for working with JSON path indices. >> >> The main difference is that tuple format fields are now organised in a >> tree rather than an array, and the tuple itself may have indexed fields, >> which are not plain array members, but rather members of some sub-array >> or map. This requires more complex iteration over tuple format fields >> and additional tuple parsing. >> >> All the changes were, however, unneeded for tuple formats not supporting >> fields indexed by JSON paths. >> >> Rework tuple_field_map_create so that it doesn't go through all the >> unnecessary JSON path-related checks for simple cases and restore most >> of the lost performance. >> >> Below are some benchmark results for the same workload that pointed to >> the degradation initially. >> Snapshot recovery time on RelWithDebInfo build for a 1.5G snapshot >> containing a single memtx space with one secondary index over 4 integer >> and 1 string field: >> >> Version | Time (s) | Difference relative to 1.10 >> ---------------------------|----------|---------------------------- >> 1.10 (the golden standard) | 28 | -/- >> 2.x (degradation) | 39 | + 39% >> 2.x (patched) | 31 | + 11% >> >> Profile shows that the main difference is in memtx_tuple_new due to >> tuple_init_field_map/tuple_field_map_create performance difference. >> >> Numbers below show cumulative time spent in tuple_init_field_map (1.10) / >> tuple_field_map_create (unpatched) / tuple_field_map_create (patched). >> 2.44 s / 8.61 s / 3.19 s >> >> More benchmark results can be seen at #4774 >> >> Part of #4774 >> >> wip. optimisation. > You may want to delete the last sentence. Thanks for the review! I do. Fixed. > > Other than that, looks good. -- Serge Petrenko