Re: [Tarantool-patches] [PATCH v2 1/3] box: introduce port_c

[Igor Munkin <imun@tarantool.org>]

Vlad,

Thanks for the patch! I left a single nit below, otherwise LGTM.

On 23.04.20, Vladislav Shpilevoy wrote:
> Port_c is a new descendant of struct port. It is used now for
> public C functions to store their result. Currently they can
> return only a tuple, but it will change soon, they will be able to
> return arbitrary MessagePack.
> 
> Port_tuple is not removed, because still is used for box_select(),
> for functional indexes, and in SQL as a base for port_sql.
> Although that may be changed later. Functional indexes really need
> only a single MessagePack object from their function. While
> box_select() working via port_tuple or port_c didn't show any
> significant difference during micro benchmarks.
> 
> Part of #4641
> ---
>  src/box/box.cc      |   2 +-
>  src/box/func.c      |   2 +-
>  src/box/lua/misc.cc |  20 +++++
>  src/box/port.c      | 206 +++++++++++++++++++++++++++++++++++++++++---
>  src/box/port.h      |  50 +++++++++++
>  src/box/sql/func.c  |  22 +++--
>  6 files changed, 281 insertions(+), 21 deletions(-)
> 

<snipped>

> diff --git a/src/box/port.c b/src/box/port.c
> index 6e2fe3a6e..2c1fadb5c 100644
> --- a/src/box/port.c
> +++ b/src/box/port.c
> @@ -37,7 +37,18 @@
>  #include <fiber.h>
>  #include "errinj.h"
>  
> -static struct mempool port_tuple_entry_pool;
> +/**
> + * The pools is used both by port_c and port_tuple, since their

Typo: s/is/are/.

> + * entires are almost of the same size. Also port_c can use
> + * objects from the pool to store result data in their memory,
> + * when it fits.
> + */
> +static struct mempool port_entry_pool;
> +
> +enum {
> +	PORT_ENTRY_SIZE = MAX(sizeof(struct port_c_entry),
> +			      sizeof(struct port_tuple_entry)),
> +};
>  
>  int
>  port_tuple_add(struct port *base, struct tuple *tuple)

<snipped>

> -- 
> 2.21.1 (Apple Git-122.3)
> 

-- 
Best regards,
IM