[patches] [PATCH 2/3] sql: remove extra key copy from cursor routine

[Alex Khatskevich]

On 12.02.2018 17:06, Nikita Pettik wrote:
> SQL cursor holds key which is used to create index iterator.  Iterator
> itself doesn't make copy of key, it only saves pointer to it.
> moveToUnpacked() allocates memory for the key on region and pass it to
> cursor positioning function, which in its turn copies it from region to
> malloc.  Other callers of this function pass only static variables as
> key arguments.  All points considered, this memory copy in cursor
> positioning function turns out to be redundant and can be removed. In
> this respect, moveToUnpacked() will allocate memory for key immediately
> using malloc.
> ---
>   src/box/sql.c | 21 ++++++++-------------
>   1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 682f15753..1ed2490c7 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -292,15 +292,17 @@ int tarantoolSqlite3MovetoUnpacked(BtCursor *pCur, UnpackedRecord *pIdxKey,
>   {
>   	int rc, res_success;
>   	size_t ks;
> -	const char *k, *ke;
> +	const char *ke;
>   	enum iterator_type iter_type;
> +	struct ta_cursor *taCur = pCur->pTaCursor;
>   
>   	ks = sqlite3VdbeMsgpackRecordLen(pIdxKey->aMem,
>   					 pIdxKey->nField);
> -	k = region_reserve(&fiber()->gc, ks);
> -	if (k == NULL) return SQLITE_NOMEM;
> -	ke = k + sqlite3VdbeMsgpackRecordPut((u8 *)k, pIdxKey->aMem,
> -					     pIdxKey->nField);
> +	taCur = cursor_create(taCur, ks);
> +	ke = taCur->key + sqlite3VdbeMsgpackRecordPut((u8 *)taCur->key,
> +						      pIdxKey->aMem,
> +						      pIdxKey->nField);
> +	pCur->pTaCursor = taCur;
>   
>   	switch (pIdxKey->opcode) {
>   	default:
> @@ -338,7 +340,7 @@ int tarantoolSqlite3MovetoUnpacked(BtCursor *pCur, UnpackedRecord *pIdxKey,
>   		res_success = 0;
>   		break;
>   	}
> -	rc = cursor_seek(pCur, pRes, iter_type, k, ke);
> +	rc = cursor_seek(pCur, pRes, iter_type, taCur->key, ke);
>   	if (*pRes == 0) {
>   		*pRes = res_success;
>   		/*
> @@ -1184,13 +1186,6 @@ cursor_seek(BtCursor *pCur, int *pRes, enum iterator_type type,
>   	}
>   	pCur->pTaCursor = c;
>   
After your change, there is no need in key and key_end attributes any more.
They are only confuse it's user.
Am I right?
> -	/* Copy key if necessary. */
> -	if (key_size != 0) {
> -		memcpy(c->key, key, key_end - key);
> -		key_end = c->key + (key_end - key);
> -		key = c->key;
> -	}
> -
>   	struct space *space;
>   	struct index *index;
>   	uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pCur->pgnoRoot);