[patches] [PATCH 3/4] tuple: check key for sequential parts on compile time

[Vladimir Davydov]

On Sat, Feb 17, 2018 at 11:35:34PM +0300, v.shpilevoy at tarantool.org wrote:
> I did not measure it with benches. But obviously removal these cycles
> on compile time is faster, than ‘for’s in each extracter.

Well, for me it's not that obvious that this patch will result in any
*noticeable* performance gain. If you want to convince me, please run
some tests and include the results in the commit log.

> > 17 февр. 2018 г., в 22:03, Vladimir Davydov <vdavydov.dev at gmail.com> написал(а):
> > 
> >> On Tue, Feb 13, 2018 at 06:28:37PM +0300, Vladislav Shpilevoy wrote:
> >> --- a/src/box/tuple_extract_key.cc
> >> +++ b/src/box/tuple_extract_key.cc
> >> @@ -57,6 +57,7 @@ tuple_extract_key_sequential(const struct tuple *tuple,
> >>  * General-purpose implementation of tuple_extract_key()
> >>  * @copydoc tuple_extract_key()
> >>  */
> >> +template <bool contains_sequential_parts>
> >> static char *
> >> tuple_extract_key_slowpath(const struct tuple *tuple,
> >>               const struct key_def *key_def, uint32_t *key_size)
> >> @@ -73,17 +74,21 @@ tuple_extract_key_slowpath(const struct tuple *tuple,
> >>            tuple_field_raw(format, data, field_map,
> >>                    key_def->parts[i].fieldno);
> >>        const char *end = field;
> >> -        /*
> >> -         * Skip sequential part in order to minimize
> >> -         * tuple_field_raw() calls.
> >> -         */
> >> -        for (; i < key_def->part_count - 1; i++) {
> >> -            if (key_def->parts[i].fieldno + 1 !=
> >> -                key_def->parts[i + 1].fieldno) {
> >> -                /* End of sequential part */
> >> -                break;
> >> +        if (contains_sequential_parts) {
> >> +            /*
> >> +             * Skip sequential part in order to
> >> +             * minimize tuple_field_raw() calls.
> >> +             */
> >> +            for (; i < key_def->part_count - 1; i++) {
> >> +                if (key_def->parts[i].fieldno + 1 !=
> >> +                    key_def->parts[i + 1].fieldno) {
> >> +                    /*
> >> +                     * End of sequential part.
> >> +                     */
> >> +                    break;
> >> +                }
> >> +                mp_next(&end);
> > 
> > In comparison to tuple_field_raw(), which is called on each iteration,
> > the cost of the loop you're optimizing out should be negligible. Is it
> > really worth it? Did you run any tests that showed a performance gain?
>