[patches] [PATCH] sql: release all region memory after VDBE's halt

[Alex Khatskevich]

Are you sure that it is impossible to localize all insertions to the 
ephemeral tables and

allocate memory for them using malloc?

It is important because of two reasons:

1. It consumes less memory during query execution

2. It is faster, because after a number of allocations with malloc, the 
Mem object have enough space

     and do not even call malloc | realloc at all, it just reuses the 
same memory.


On 28.02.2018 23:15, Nikita Pettik wrote:
> When ticket #3021 was implemented, alongside with it leak of region
> memory was introduced. It occured due to allocation memory for tuples at
> region at once (previously, it was allocated using malloc and before
> insertion to space it was copied to the region).  However, if space is
> ephemeral, it has nothing in common with txn routine. As a result,
> region memory for tuples to be inserted in ephemeral spaces isn't pinned
> to any txn and isn't freed by txn_commit() or txn_rollback(). To avoid
> this leak, now  all used region memory is released in vdbe's halt
> routine.
>
> Closes #3199
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3199-mem-leak-fix
> Issue: https://github.com/tarantool/tarantool/issues/3199
>
>   src/box/sql/vdbe.c                     |   8 +++
>   src/box/sql/vdbeaux.c                  |   6 ++
>   test/sql/gh-3199-no-mem-leaks.result   | 120 +++++++++++++++++++++++++++++++++
>   test/sql/gh-3199-no-mem-leaks.test.lua |  46 +++++++++++++
>   4 files changed, 180 insertions(+)
>   create mode 100644 test/sql/gh-3199-no-mem-leaks.result
>   create mode 100644 test/sql/gh-3199-no-mem-leaks.test.lua
>
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index b235e7bf6..a289b35a5 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2750,6 +2750,14 @@ case OP_MakeRecord: {
>   	 * statement commitment, it is better to reuse the same chunk.
>   	 * Such optimization is prohibited for ordinary spaces, since
>   	 * memory shouldn't be reused until it is written into WAL.
> +	 *
> +	 * However, if memory for ephemeral space is allocated
> +	 * on region, it will be freed only in vdbeHalt() routine.
> +	 * It is the only way to free this region memory,
> +	 * since ephemeral spaces don't have nothing in common
> +	 * with txn routine and region memory won't be released
> +	 * after txn_commit() or txn_rollback() as it happens
> +	 * with ordinary spaces.
>   	 */
>   	if (bIsEphemeral) {
>   		rc = sqlite3VdbeMemClearAndResize(pOut, nByte);
> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
> index d7546ab43..a5f8b6eca 100644
> --- a/src/box/sql/vdbeaux.c
> +++ b/src/box/sql/vdbeaux.c
> @@ -2771,6 +2771,12 @@ sqlite3VdbeHalt(Vdbe * p)
>   		p->rc = SQLITE_NOMEM_BKPT;
>   	}
>   
> +	/* Release all region memory which was allocated
> +	 * to hold tuples to be inserted into ephemeral spaces.
> +	 */
> +	if (!box_txn())
> +		fiber_gc();
> +
>   	assert(db->nVdbeActive > 0 || p->autoCommit == 0 ||
>   		       p->anonymous_savepoint == NULL);
>   	return (p->rc == SQLITE_BUSY ? SQLITE_BUSY : SQLITE_OK);
> diff --git a/test/sql/gh-3199-no-mem-leaks.result b/test/sql/gh-3199-no-mem-leaks.result
> new file mode 100644
> index 000000000..682ca62a7
> --- /dev/null
> +++ b/test/sql/gh-3199-no-mem-leaks.result
> @@ -0,0 +1,120 @@
> +test_run = require('test_run').new()
> +---
> +...
> +fiber = require('fiber')
> +---
> +...
> +-- This test checks that no leaks of region memory happens during
> +-- executing SQL queries.
> +--
> +-- box.cfg()
> +box.sql.execute('CREATE TABLE test (id PRIMARY KEY, x INTEGER, y INTEGER)')
> +---
> +...
> +box.sql.execute('INSERT INTO test VALUES (1, 1, 1), (2, 2, 2)')
> +---
> +...
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +---
> +- - [1, 1, 2]
> +  - [2, 2, 4]
> +...
> +fiber.info()[fiber.self().id()].memory.used
> +---
> +- 0
> +...
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +---
> +- - [1, 1, 2]
> +  - [2, 2, 4]
> +...
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +---
> +- - [1, 1, 2]
> +  - [2, 2, 4]
> +...
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +---
> +- - [1, 1, 2]
> +  - [2, 2, 4]
> +...
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +---
> +- - [1, 1, 2]
> +  - [2, 2, 4]
> +...
> +fiber.info()[fiber.self().id()].memory.used
> +---
> +- 0
> +...
> +box.sql.execute('CREATE TABLE test2 (id PRIMARY KEY, a TEXT, b INTEGER)')
> +---
> +...
> +box.sql.execute('INSERT INTO test2 VALUES (1, \'abc\', 1), (2, \'hello\', 2)')
> +---
> +...
> +box.sql.execute('INSERT INTO test2 VALUES (3, \'test\', 3), (4, \'xx\', 4)')
> +---
> +...
> +box.sql.execute('SELECT a, id + 2 * a, b FROM test2 WHERE b < id * 2 ORDER BY a ')
> +---
> +- - ['abc', 1, 1]
> +  - ['hello', 2, 2]
> +  - ['test', 3, 3]
> +  - ['xx', 4, 4]
> +...
> +fiber.info()[fiber.self().id()].memory.used
> +---
> +- 0
> +...
> +box.sql.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
> +---
> +- - ['abc', 3, 'abc']
> +  - ['hello', 6, 'hello']
> +  - ['test', 9, 'test']
> +  - ['xx', 12, 'xx']
> +...
> +box.sql.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
> +---
> +- - ['abc', 3, 'abc']
> +  - ['hello', 6, 'hello']
> +  - ['test', 9, 'test']
> +  - ['xx', 12, 'xx']
> +...
> +box.sql.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
> +---
> +- - ['abc', 3, 'abc']
> +  - ['hello', 6, 'hello']
> +  - ['test', 9, 'test']
> +  - ['xx', 12, 'xx']
> +...
> +fiber.info()[fiber.self().id()].memory.used
> +---
> +- 0
> +...
> +box.sql.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x')
> +---
> +- - [1, 4, 1]
> +  - [2, 8, 2]
> +...
> +box.sql.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x')
> +---
> +- - [1, 4, 1]
> +  - [2, 8, 2]
> +...
> +box.sql.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x')
> +---
> +- - [1, 4, 1]
> +  - [2, 8, 2]
> +...
> +fiber.info()[fiber.self().id()].memory.used
> +---
> +- 0
> +...
> +-- Cleanup
> +box.sql.execute('DROP TABLE test')
> +---
> +...
> +box.sql.execute('DROP TABLE test2')
> +---
> +...
> diff --git a/test/sql/gh-3199-no-mem-leaks.test.lua b/test/sql/gh-3199-no-mem-leaks.test.lua
> new file mode 100644
> index 000000000..d61d474d9
> --- /dev/null
> +++ b/test/sql/gh-3199-no-mem-leaks.test.lua
> @@ -0,0 +1,46 @@
> +test_run = require('test_run').new()
> +fiber = require('fiber')
> +
> +-- This test checks that no leaks of region memory happens during
> +-- executing SQL queries.
> +--
> +
> +-- box.cfg()
> +
> +
> +box.sql.execute('CREATE TABLE test (id PRIMARY KEY, x INTEGER, y INTEGER)')
> +box.sql.execute('INSERT INTO test VALUES (1, 1, 1), (2, 2, 2)')
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +
> +fiber.info()[fiber.self().id()].memory.used
> +
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +box.sql.execute('SELECT x, y, x + y FROM test ORDER BY y')
> +
> +fiber.info()[fiber.self().id()].memory.used
> +
> +box.sql.execute('CREATE TABLE test2 (id PRIMARY KEY, a TEXT, b INTEGER)')
> +box.sql.execute('INSERT INTO test2 VALUES (1, \'abc\', 1), (2, \'hello\', 2)')
> +box.sql.execute('INSERT INTO test2 VALUES (3, \'test\', 3), (4, \'xx\', 4)')
> +box.sql.execute('SELECT a, id + 2 * a, b FROM test2 WHERE b < id * 2 ORDER BY a ')
> +
> +fiber.info()[fiber.self().id()].memory.used
> +
> +box.sql.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
> +box.sql.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
> +box.sql.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
> +
> +fiber.info()[fiber.self().id()].memory.used
> +
> +box.sql.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x')
> +box.sql.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x')
> +box.sql.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x')
> +
> +fiber.info()[fiber.self().id()].memory.used
> +
> +-- Cleanup
> +box.sql.execute('DROP TABLE test')
> +box.sql.execute('DROP TABLE test2')
> +