Skip to content

Commit

Permalink
Use unordered_map instead of my_hash
Browse files Browse the repository at this point in the history
Upstream commit ID : fb-mysql-5.6.35/1023429b32fd596722497d4fad576a4aecc1539b
PS-6867 : Merge fb-prod201905

Summary:
This is part of MyRocks 8.0 prep work - front loading as much work as possible in 5.6 before we actually port MyRocks to 8.0.

In MySQL 8.0 the my_hash completely stuff is gone and replaced by unordered_map and various sub-class (such as collation_unordered_map, etc). And in their source code blog they claim that by switching to unordered_map gives them performance wins. So it makes sense for us to switch to it preemptively before we port to MySQL 8.0.

This is rewrite/enhancements of below changes from georgelorchpercona (not a literal port):

facebook/mysql-5.6@20d4b2b
facebook/mysql-5.6@8a5e758

Reviewed By: lth

Differential Revision: D15313077

fbshipit-source-id: fb65bf27de0
  • Loading branch information
yizhang82 authored and oleksandr-kachan committed Jan 19, 2024
1 parent e98ebf0 commit 0d24207
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 38 deletions.
70 changes: 48 additions & 22 deletions storage/rocksdb/ha_rocksdb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,27 @@ static void rocksdb_flush_all_memtables() {
namespace // anonymous namespace = not visible outside this source file
{

struct Rdb_open_tables_map {
class Rdb_open_tables_map {
private:
/* Hash table used to track the handlers of open tables */
std::unordered_map<std::string, Rdb_table_handler *> m_hash;
std::unordered_map<std::string, Rdb_table_handler *> m_table_map;

/* The mutex used to protect the hash table */
mutable mysql_mutex_t m_mutex;

public:
void init() {
m_table_map.clear();
mysql_mutex_init(rdb_psi_open_tbls_mutex_key, &m_mutex, MY_MUTEX_INIT_FAST);
}

void free() {
m_table_map.clear();
mysql_mutex_destroy(&m_mutex);
}

size_t count() { return m_table_map.size(); }

Rdb_table_handler *get_table_handler(const char *const table_name);
void release_table_handler(Rdb_table_handler *const table_handler);

Expand Down Expand Up @@ -1984,6 +1999,8 @@ static int rocksdb_compact_column_family(THD *const thd,
return HA_EXIT_SUCCESS;
}

///////////////////////////////////////////////////////////////////////////////////////////

/*
Drop index thread's control
*/
Expand Down Expand Up @@ -4568,8 +4585,10 @@ static int rocksdb_init_func(void *const p) {
init_rocksdb_psi_keys();

rocksdb_hton = (handlerton *)p;
mysql_mutex_init(rdb_psi_open_tbls_mutex_key, &rdb_open_tables.m_mutex,
MY_MUTEX_INIT_FAST);

rdb_open_tables.init();
Ensure_cleanup rdb_open_tables_cleanup([]() { rdb_open_tables.free(); });

#ifdef HAVE_PSI_INTERFACE
rdb_bg_thread.init(rdb_signal_bg_psi_mutex_key, rdb_signal_bg_psi_cond_key);
rdb_drop_idx_thread.init(rdb_signal_drop_idx_psi_mutex_key,
Expand Down Expand Up @@ -5022,6 +5041,10 @@ static int rocksdb_init_func(void *const p) {
rdb_get_hton_init_state()->set_initialized(true);

LogPluginErrMsg(INFORMATION_LEVEL, 0, "instance opened");

// Skip cleaning up rdb_open_tables as we've succeeded
rdb_open_tables_cleanup.skip();

DBUG_RETURN(HA_EXIT_SUCCESS);
}

Expand Down Expand Up @@ -5083,13 +5106,13 @@ static int rocksdb_done_func(void *const p) {
err);
}

if (rdb_open_tables.m_hash.size()) {
if (rdb_open_tables.count()) {
// Looks like we are getting unloaded and yet we have some open tables
// left behind.
error = 1;
}

mysql_mutex_destroy(&rdb_open_tables.m_mutex);
rdb_open_tables.free();
mysql_mutex_destroy(&rdb_sysvars_mutex);
mysql_mutex_destroy(&rdb_block_cache_resize_mutex);

Expand Down Expand Up @@ -5196,40 +5219,43 @@ Rdb_open_tables_map::get_table_handler(const char *const table_name) {

DBUG_ASSERT(table_name != nullptr);

const std::string s_table_name(table_name);
Rdb_table_handler *table_handler;

const std::string table_name_str(table_name);

// First, look up the table in the hash map.
RDB_MUTEX_LOCK_CHECK(m_mutex);
Rdb_table_handler *table_handler = nullptr;
const auto &it = m_hash.find(s_table_name);
if (it != m_hash.end()) {
const auto &it = m_table_map.find(table_name_str);
if (it != m_table_map.end()) {
// Found it
table_handler = it->second;
} else {
char *tmp_name;

// Since we did not find it in the hash map, attempt to create and add it
// to the hash map.
char *tmp_name;
#ifdef HAVE_PSI_INTERFACE
if (!(table_handler = static_cast<Rdb_table_handler *>(
if (!(table_handler = reinterpret_cast<Rdb_table_handler *>(
my_multi_malloc(rdb_handler_memory_key, MYF(MY_WME | MY_ZEROFILL),
&table_handler, sizeof(*table_handler), &tmp_name,
s_table_name.length() + 1, NullS)))) {
table_name_str.length() + 1, NullS)))) {
#else
if (!(table_handler = static_cast<Rdb_table_handler *>(
if (!(table_handler = reinterpret_cast<Rdb_table_handler *>(
my_multi_malloc(PSI_NOT_INSTRUMENTED, MYF(MY_WME | MY_ZEROFILL),
&table_handler, sizeof(*table_handler), &tmp_name,
s_table_name.length() + 1, NullS)))) {
table_name_str.length() + 1, NullS)))) {
#endif
// Allocating a new Rdb_table_handler and a new table name failed.
RDB_MUTEX_UNLOCK_CHECK(m_mutex);
return nullptr;
}

table_handler->m_ref_count = 0;
table_handler->m_table_name_length = s_table_name.length();
table_handler->m_table_name_length = table_name_str.length();
table_handler->m_table_name = tmp_name;
my_stpmov(table_handler->m_table_name, s_table_name.c_str());
my_stpmov(table_handler->m_table_name, table_name_str.c_str());

m_hash.insert({s_table_name, table_handler});
m_table_map.emplace(table_name_str, table_handler);

thr_lock_init(&table_handler->m_thr_lock);
table_handler->m_io_perf_read.init();
Expand All @@ -5251,8 +5277,8 @@ std::vector<std::string> Rdb_open_tables_map::get_table_names(void) const {
std::vector<std::string> names;

RDB_MUTEX_LOCK_CHECK(m_mutex);
for (const auto &it : m_hash) {
table_handler = it.second;
for (const auto &kv : m_table_map) {
table_handler = kv.second;
DBUG_ASSERT(table_handler != nullptr);
names.push_back(table_handler->m_table_name);
}
Expand Down Expand Up @@ -5525,8 +5551,8 @@ void Rdb_open_tables_map::release_table_handler(
DBUG_ASSERT(table_handler->m_ref_count > 0);
if (!--table_handler->m_ref_count) {
const auto ret MY_ATTRIBUTE((__unused__)) =
m_hash.erase(std::string(table_handler->m_table_name));
DBUG_ASSERT(ret == 1);
m_table_map.erase(std::string(table_handler->m_table_name));
DBUG_ASSERT(ret == 1); // the hash entry must actually be found and deleted
my_core::thr_lock_delete(&table_handler->m_thr_lock);
my_free(table_handler);
}
Expand Down
29 changes: 16 additions & 13 deletions storage/rocksdb/rdb_datadic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4777,7 +4777,7 @@ bool Rdb_ddl_manager::init(Rdb_dict_manager *const dict_arg,

Rdb_tbl_def *Rdb_ddl_manager::find(const std::string &table_name,
const bool lock) {
Rdb_tbl_def *ret = nullptr;
Rdb_tbl_def *rec = nullptr;

if (lock) {
mysql_rwlock_rdlock(&m_rwlock);
Expand All @@ -4791,7 +4791,7 @@ Rdb_tbl_def *Rdb_ddl_manager::find(const std::string &table_name,
mysql_rwlock_unlock(&m_rwlock);
}

return ret;
return rec;
}

// this is a safe version of the find() function below. It acquires a read
Expand Down Expand Up @@ -4947,25 +4947,26 @@ int Rdb_ddl_manager::put_and_write(Rdb_tbl_def *const tbl,

/* Return 0 - ok, other value - error */
/* TODO:
This function modifies m_ddl_hash and m_index_num_to_keydef.
This function modifies m_ddl_map and m_index_num_to_keydef.
However, these changes need to be reversed if dict_manager.commit fails
See the discussion here: https://reviews.facebook.net/D35925#inline-259167
Tracked by https://github.com/facebook/mysql-5.6/issues/33
*/
int Rdb_ddl_manager::put(Rdb_tbl_def *const tbl, const bool lock) {
Rdb_tbl_def *rec;
const std::string &dbname_tablename = tbl->full_tablename();

if (lock)
mysql_rwlock_wrlock(&m_rwlock);

// We have to do this find because 'tbl' is not yet in the list. We need
// to find the one we are replacing ('rec')
const auto &it = m_ddl_hash.find(dbname_tablename);
if (it != m_ddl_hash.end()) {
const auto &it = m_ddl_map.find(dbname_tablename);
if (it != m_ddl_map.end()) {
delete it->second;
m_ddl_hash.erase(it);
m_ddl_map.erase(it);
}
m_ddl_hash.insert({dbname_tablename, tbl});
m_ddl_map.emplace(dbname_tablename, tbl);

for (uint keyno = 0; keyno < tbl->m_key_count; keyno++) {
m_index_num_to_keydef[tbl->m_key_descr_arr[keyno]->get_gl_index_id()] =
Expand All @@ -4991,10 +4992,12 @@ void Rdb_ddl_manager::remove(Rdb_tbl_def *const tbl,

m_dict->delete_key(batch, key_writer.to_slice());

const auto &it = m_ddl_hash.find(dbname_tablename);
if (it != m_ddl_hash.end()) {
const auto &it = m_ddl_map.find(dbname_tablename);
if (it != m_ddl_map.end()) {
// Free Rdb_tbl_def
delete it->second;
m_ddl_hash.erase(it);

m_ddl_map.erase(it);
}

if (lock)
Expand Down Expand Up @@ -5055,7 +5058,7 @@ void Rdb_ddl_manager::cleanup() {
}

int Rdb_ddl_manager::scan_for_tables(Rdb_tables_scanner *const tables_scanner) {
int i, ret;
int ret;

DBUG_ASSERT(tables_scanner != nullptr);

Expand All @@ -5064,8 +5067,8 @@ int Rdb_ddl_manager::scan_for_tables(Rdb_tables_scanner *const tables_scanner) {
ret = 0;
i = 0;

for (const auto &it : m_ddl_hash) {
ret = tables_scanner->add_table(it.second);
for (const auto &kv : m_ddl_map) {
ret = tables_scanner->add_table(kv.second);
if (ret)
break;
i++;
Expand Down
5 changes: 4 additions & 1 deletion storage/rocksdb/rdb_datadic.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <map>
#include <mutex>
#include <string>
#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -1242,8 +1243,10 @@ interface Rdb_tables_scanner {

class Rdb_ddl_manager {
Rdb_dict_manager *m_dict = nullptr;

// Contains Rdb_tbl_def elements
std::unordered_map<std::string, Rdb_tbl_def *> m_ddl_hash;
std::unordered_map<std::string, Rdb_tbl_def *> m_ddl_map;

// Maps index id to <table_name, index number>
std::map<GL_INDEX_ID, std::pair<std::string, uint>> m_index_num_to_keydef;

Expand Down
13 changes: 11 additions & 2 deletions storage/rocksdb/rdb_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,20 @@ std::vector<std::string> split_into_vector(const std::string &input,
*/
class Ensure_cleanup {
public:
explicit Ensure_cleanup(std::function<void()> cleanup) : m_cleanup(cleanup) {}
explicit Ensure_cleanup(std::function<void()> cleanup)
: m_cleanup(cleanup), m_skip_cleanup(false) {}

~Ensure_cleanup() { m_cleanup(); }
~Ensure_cleanup() {
if (!m_skip_cleanup) {
m_cleanup();
}
}

// If you want to skip cleanup (such as when the operation is successful)
void skip() { m_skip_cleanup = true; }

private:
std::function<void()> m_cleanup;
bool m_skip_cleanup;
};
} // namespace myrocks

0 comments on commit 0d24207

Please sign in to comment.