[tarantool-patches] Re: [PATCH 0/8] Get rid of Vinyl's mem_format_with_colmask

[Vladimir Davydov]

On Tue, Oct 23, 2018 at 10:32:16AM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [18/10/15 10:15]:
> > There are two formats for memory statements in Vinyl, vy_lsm::mem_format
> > and vy_lsm::mem_format_with_colmask. The only difference between them is
> > that the latter allows to store column mask inside tuples. It is used to
> > optimize out tautological updates in the write iterator. Turns out that
> > we can easily do without it as this patch set demonstrates.
> > 
> > I've always wanted to drop this format because I hate the very idea of
> > tuple_extra (that's why I dropped upsert format a while ago as well).
> > However, the reason why I'm doing this now is that it is the only thing
> > why we need tuple_format_dup. Currently, tuple_format_dup is basically a
> > memcpy, but once JSON indexes are introduced it's going to become much
> > more complex than just that so we'd better drop it now.
> 
> First, I disagree with both premises:
> 
> - we will need tuple_extra anyway, for capped collections

I won't, because tuple_extra is extremely inconvenient to use. I will
inherit memtx_tuple instead and access extra fields after casting tuple
to the child class. This will look much easier for understanding.

> - you can't guarantee that a pretty basic method of an object such as creating 
>   a deep copy will never be used in the future, so there is no
>   much point in attacking it now.

Why would we need to make a deep copy of a tuple_format, even
theoretically, when we can simply reference it? Note, tuple_format
is immutable by design.

> 
> Yet I am happy to look at the patch trimming the code on the edges so a review
> follows.