Skip to content

Commit 59f1fb8

Browse files
MyRocks: merge the key packing buffer allocation, annotate m_pack_buffer lifetime
All the key packing buffers in the MyRocks handler have exactly the same lifetime between alloc_key_buffers and free_key_buffers. Thus allocate a single large buffer and point the key packing buffers to offsets inside it, reducing number of calls to the heap allocator on table opening/close code path. Annotate the padding bytes for Valgrind as inaccessible. Add additional handling for m_pack_buffer: since its use it's fully contained in Rdb_key_def::pack_record, annotate it as such.
1 parent 6aba81d commit 59f1fb8

File tree

3 files changed

+60
-35
lines changed

3 files changed

+60
-35
lines changed

storage/rocksdb/ha_rocksdb.cc

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9379,15 +9379,22 @@ int ha_rocksdb::convert_record_from_storage_format(
93799379
: rc;
93809380
}
93819381

9382+
static constexpr std::size_t get_next_aligned(std::size_t &offset) {
9383+
constexpr auto bits_below_alignment = alignof(std::max_align_t) - 1;
9384+
constexpr auto bits_above_alignment = ~bits_below_alignment;
9385+
9386+
const auto padding =
9387+
((offset + bits_below_alignment) & bits_above_alignment) - offset;
9388+
offset += padding;
9389+
9390+
return padding;
9391+
}
9392+
93829393
int ha_rocksdb::alloc_key_buffers(const TABLE &table_arg,
93839394
const Rdb_tbl_def &tbl_def_arg) {
93849395
DBUG_ENTER_FUNC();
93859396

93869397
std::shared_ptr<Rdb_key_def> *const kd_arr = tbl_def_arg.m_key_descr_arr;
9387-
9388-
uint max_packed_sk_len = 0;
9389-
uint pack_key_len = 0;
9390-
93919398
m_pk_descr = kd_arr[pk_index(table_arg, tbl_def_arg)];
93929399

93939400
// move this into get_table_handler() ??
@@ -9396,12 +9403,11 @@ int ha_rocksdb::alloc_key_buffers(const TABLE &table_arg,
93969403
return rtn;
93979404
}
93989405

9399-
pack_key_len = m_pk_descr->max_storage_fmt_length();
9400-
m_pk_packed_tuple = reinterpret_cast<uchar *>(
9401-
my_malloc(PSI_NOT_INSTRUMENTED, pack_key_len, MYF(0)));
9406+
const auto pack_key_len = m_pk_descr->max_storage_fmt_length();
9407+
auto buf_size = static_cast<std::size_t>(pack_key_len);
94029408

94039409
/* Sometimes, we may use m_sk_packed_tuple for storing packed PK */
9404-
max_packed_sk_len = pack_key_len;
9410+
auto max_packed_sk_len = pack_key_len;
94059411
for (uint i = 0; i < table_arg.s->keys; i++) {
94069412
/* Primary key was processed above */
94079413
if (i == table_arg.s->primary_key) continue;
@@ -9418,48 +9424,57 @@ int ha_rocksdb::alloc_key_buffers(const TABLE &table_arg,
94189424
}
94199425
}
94209426

9421-
m_sk_packed_tuple = reinterpret_cast<uchar *>(
9422-
my_malloc(PSI_NOT_INSTRUMENTED, max_packed_sk_len, MYF(0)));
9423-
m_sk_packed_tuple_old = reinterpret_cast<uchar *>(
9424-
my_malloc(PSI_NOT_INSTRUMENTED, max_packed_sk_len, MYF(0)));
9425-
m_sk_packed_tuple_updated = reinterpret_cast<uchar *>(
9426-
my_malloc(PSI_NOT_INSTRUMENTED, max_packed_sk_len, MYF(0)));
9427-
m_end_key_packed_tuple = reinterpret_cast<uchar *>(
9428-
my_malloc(PSI_NOT_INSTRUMENTED, max_packed_sk_len, MYF(0)));
9429-
m_pack_buffer = reinterpret_cast<uchar *>(
9430-
my_malloc(PSI_NOT_INSTRUMENTED, max_packed_sk_len, MYF(0)));
9427+
const auto pad1 [[maybe_unused]] = get_next_aligned(buf_size);
9428+
const auto m_sk_packed_tuple_offset = buf_size;
9429+
buf_size += max_packed_sk_len;
9430+
9431+
const auto pad2 [[maybe_unused]] = get_next_aligned(buf_size);
9432+
const auto m_sk_packed_tuple_old_offset = buf_size;
9433+
buf_size += max_packed_sk_len;
9434+
9435+
const auto pad3 [[maybe_unused]] = get_next_aligned(buf_size);
9436+
const auto m_sk_packed_tuple_updated_offset = buf_size;
9437+
buf_size += max_packed_sk_len;
94319438

9432-
if (m_pk_packed_tuple == nullptr || m_sk_packed_tuple == nullptr ||
9433-
m_sk_packed_tuple_old == nullptr ||
9434-
m_sk_packed_tuple_updated == nullptr ||
9435-
m_end_key_packed_tuple == nullptr || m_pack_buffer == nullptr) {
9436-
// One or more of the above allocations failed. Clean up and exit
9439+
const auto pad4 [[maybe_unused]] = get_next_aligned(buf_size);
9440+
const auto m_end_key_packed_tuple_offset = buf_size;
9441+
buf_size += max_packed_sk_len;
9442+
9443+
const auto pad5 [[maybe_unused]] = get_next_aligned(buf_size);
9444+
const auto m_pack_buffer_offset = buf_size;
9445+
buf_size += max_packed_sk_len;
9446+
9447+
buffers.reset(static_cast<uchar *>(
9448+
my_malloc(PSI_NOT_INSTRUMENTED, buf_size, MYF(0))));
9449+
if (buffers == nullptr) {
94379450
free_key_buffers();
94389451

94399452
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
94409453
}
94419454

9455+
m_pk_packed_tuple = buffers.get();
9456+
MEM_NOACCESS(m_pk_packed_tuple + pack_key_len, pad1);
9457+
m_sk_packed_tuple = buffers.get() + m_sk_packed_tuple_offset;
9458+
MEM_NOACCESS(m_sk_packed_tuple + max_packed_sk_len, pad2);
9459+
m_sk_packed_tuple_old = buffers.get() + m_sk_packed_tuple_old_offset;
9460+
MEM_NOACCESS(m_sk_packed_tuple_old + max_packed_sk_len, pad3);
9461+
m_sk_packed_tuple_updated = buffers.get() + m_sk_packed_tuple_updated_offset;
9462+
MEM_NOACCESS(m_sk_packed_tuple_updated + max_packed_sk_len, pad4);
9463+
m_end_key_packed_tuple = buffers.get() + m_end_key_packed_tuple_offset;
9464+
MEM_NOACCESS(m_end_key_packed_tuple + max_packed_sk_len, pad5);
9465+
m_pack_buffer = buffers.get() + m_pack_buffer_offset;
9466+
94429467
DBUG_RETURN(HA_EXIT_SUCCESS);
94439468
}
94449469

94459470
void ha_rocksdb::free_key_buffers() {
9446-
my_free(m_pk_packed_tuple);
94479471
m_pk_packed_tuple = nullptr;
9448-
9449-
my_free(m_sk_packed_tuple);
94509472
m_sk_packed_tuple = nullptr;
9451-
9452-
my_free(m_sk_packed_tuple_old);
94539473
m_sk_packed_tuple_old = nullptr;
9454-
9455-
my_free(m_sk_packed_tuple_updated);
94569474
m_sk_packed_tuple_updated = nullptr;
9457-
9458-
my_free(m_end_key_packed_tuple);
94599475
m_end_key_packed_tuple = nullptr;
9460-
9461-
my_free(m_pack_buffer);
94629476
m_pack_buffer = nullptr;
9477+
buffers.reset();
94639478

94649479
release_blob_buffer();
94659480
}

storage/rocksdb/ha_rocksdb.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,14 @@ class ha_rocksdb : public my_core::handler, public blob_buffer {
194194
*/
195195
mutable bool m_pk_can_be_decoded;
196196

197+
// The common buffer for m_pk_packed_tuple, m_sk_packed_tuple,
198+
// m_sk_packed_tuple_old, m_sk_packed_tuple_updated, m_end_key_packed_tuple,
199+
// & m_pack_buffer.
200+
unique_ptr_my_free<uchar[]> buffers;
201+
// ^^ todo: change it to 'char[]'?
202+
197203
uchar *m_pk_packed_tuple; /* Buffer for storing PK in StorageFormat */
198-
// ^^ todo: change it to 'char*'? TODO: ^ can we join this with last_rowkey?
204+
// TODO: ^ can we join this with last_rowkey?
199205

200206
/*
201207
Temporary buffers for storing the key part of the Key/Value pair

storage/rocksdb/rdb_datadic.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,6 +1320,8 @@ uint Rdb_key_def::pack_record(const TABLE *const tbl, uchar *const pack_buffer,
13201320
assert_IMP(should_store_row_debug_checksums,
13211321
(m_index_type == INDEX_TYPE_SECONDARY));
13221322

1323+
MEM_UNDEFINED(pack_buffer, max_storage_fmt_length());
1324+
13231325
uchar *tuple = packed_tuple;
13241326
size_t unpack_start_pos = size_t(-1);
13251327
size_t unpack_len_pos = size_t(-1);
@@ -1471,6 +1473,8 @@ uint Rdb_key_def::pack_record(const TABLE *const tbl, uchar *const pack_buffer,
14711473

14721474
assert(is_storage_available(tuple - packed_tuple, 0));
14731475

1476+
MEM_NOACCESS(pack_buffer, max_storage_fmt_length());
1477+
14741478
return tuple - packed_tuple;
14751479
}
14761480

0 commit comments

Comments
 (0)