coverage: 87.639%. remained the same
when pulling 8ab02e6 on Astronomax:ghe-1043-read-view-close-perf
into 49eea0e
on tarantool:master.

Let's consider the following benchmark:

local log = require('log')
local clock = require('clock')

local field_count = tonumber(arg[1])
local test_iters = tonumber(arg[2])

os.execute('rm -rf *.snap *.xlog')
box.cfg{}

local format = {}
for i = 1, field_count do
    table.insert(format, {name = 'field'..i, type = 'uint64'})
end

box.schema.space.create('test', {
    engine = 'memcs',
    format = format,
    field_count = field_count,
})

local rv_func = function()
    local a = box.read_view.open()
    a:close()
end

local tm = clock.bench(function() for i = 1, test_iters do rv_func() end end)[1]
log.info(string.format("field count: %d, sec: %f", field_count, tm / test_iters))
os.exit()

Let's examine the open + close time of a single read view against field_count. The relationship appears piecewise linear. The most interesting features here are the three sharp performance jumps (slightly to the right of 2000, 4000, and 8000; in fact, this pattern appears near every power of 2, at least up to and including 256):

Please disregard that this graph and all subsequent ones say "Tarantool read_view.open() benchmark"; this is a typo. In reality, this is an open + close benchmark (exactly the script provided at the beginning).

As noted correctly by Gumix, most of the time (approximately 80-90% depending on field_count) is spent in tuple_dictionary_delete_hash.

This function is called approximately the same number of times regardless of field_count. The issue appears to be specifically in the relationship between the execution time of a single call and field_count.

A careful examination of tuple_dictionary_delete_hash quickly raises the question: why are we manually removing each element from the hashmap at all, if we immediately call mh_strnu32_delete afterward? All elements are stored in the hashmap inplace, so del doesn't invoke any memory deallocation functions. del also doesn't call destructors for the elements; it merely sets a specific bit in the bitmap to 0. After that, mh_strnu32_delete will delete the hashmap along with the array p containing all elements inplace.

It seems we could completely remove this loop and just keep a single call to mh_strnu32_delete? The second commit implements exactly that.

After the second commit, the contribution of lbox_read_view_close becomes so insignificant that it doesn't even appear on the flamegraph.

Nit. Let's mention that mh_first() has O(n) complexity to clarify why this loop take so much time.

Successfully created backport PR for release/3.2:

• [backport 3.2] read view closing performance improvement #12123

Successfully created backport PR for release/3.3:

• [backport 3.3] read view closing performance improvement #12124

Successfully created backport PR for release/3.5:

• [backport 3.5] read view closing performance improvement #12125

Backport summary

• Created [backport 3.5] read view closing performance improvement #12125 to release/3.5 to a future 3.5.2 release
• Created [backport 3.3] read view closing performance improvement #12124 to release/3.3 to a future 3.3.5 release
• Created [backport 3.2] read view closing performance improvement #12123 to release/3.2 to a future 3.2.4 release